Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User metadata annotations #1183

Merged
merged 22 commits into from
Nov 1, 2023
Merged

User metadata annotations #1183

merged 22 commits into from
Nov 1, 2023

Conversation

mattdailis
Copy link
Collaborator

@mattdailis mattdailis commented Oct 5, 2023

Description

This is our second attempt. Our first attempt was #1055.

Short term goal: Allow users to provide unit information for parameters, computed attributes, and resources in their mission model, such that this information can be displayed in the UI and extracted via the API.

Broader goal: Set up a pattern that missions, and Aerie developers, can use to add more kinds of metadata in the future, e.g. Range.

There are two (mostly) independent problems to solve here:

  1. How do mission modelers provide unit information, and define other kinds of custom metadata?
  2. How is this custom metadata serialized and represented to downstream tools?

The following sections delve into these problems:

How to define custom metadata

We knew from the outset that we would eventually want to be able to annotate deeply nested aspects of a value schema with metadata. E.g. a rover's pose could be characterized by a compound piece of data:

pose:
  position:
    latitude: degrees
    longitude: degrees
    elevation: kilometers
  orientation: quaternion
    q1: unit vector component
    q2: unit vector component
    q3: unit vector component
    q4: scalar

We had originally descoped this capability, but then pivoted (i.e. we increased scope) when the path to implementing it became clear.

The two options to select between were Javadoc tags, and annotations. I will describe both approaches, and explain why we have settled on annotations.

Surface Syntax

This Javadoc tags approach amounted to parsing javadoc comments as strings, and extracting portions of them based on a set of tags, such as @aerie.unit.

It lended itself quite well to class style parameters, since it could be part of the javadoc comment attached to the parameter itself:

Javadoc tags approach

Example of using javadoc to add unit information to a parameter:

@ActivityType("RunHeater")
public class RunHeater {
  /**
   * @aerie.unit seconds
   */
  @Parameter
  public long duration;

  /**
   * @aerie.unit kWh
   */
  @Parameter
  public int energyConsumptionRate;
}

Example of using javadoc to add unit information to a computed attributes record:

  /**
   *
   * @aerie.computedAttribute durationInSeconds
   * @aerie.unit seconds
   * @aerie.computedAttribute energyConsumptionRate
   * @aerie.unit kWh
   */
  @AutoValueMapper.Record
  record ComputedAttributes(
    long durationInSeconds,
    int energyConsumptionRate
  ) {}

Example of using @aerie.resourceName to assign a unit to all resources that match a regular expression:

/*
 * @aerie.resourceName \/prefix/.*
 * @aerie.unit F resources
 */

A limitation that we discovered of parsing the javadocs is that our implementation would only parse javadocs in a known set of locations. This works fine for parameters and computed attributes, but it breaks down for resources, which in practice are defined in submodels.

The second approach we pursued was to introduce a @Unit annotation, which could be attached to the type of a parameter or computed attribute:

Annotation approach

Example of using an annotation to add unit information to a parameter:

@ActivityType("RunHeater")
public class RunHeater {
  @Parameter
  public @Unit("seconds") long duration;

  @Parameter
  public @Unit("kWh") int energyConsumptionRate;
}

Example of using an annotation to add unit information to a computed attributes record:

@AutoValueMapper.Record
record ComputedAttributes(
  @Unit("seconds") long durationInSeconds,
  @Unit("kWh") int energyConsumptionRate
) {}

What about resources? Those are defined through a different process from parameters and computed attributes. Resources get registered at runtime, in the mission model constructor, rather than at compile time like the other entities. The approach with javadocs would record a regular expression and do name matching at runtime. The current approach instead expects a mission model to provide the units to the registrar as runtime values. Two convenience methods have been defined:

discreteResource(registrar, "/peel", this.peel, new DoubleValueMapper(), "kg");
realResource(registrar, "/fruit", this.fruit, "bananas");

This approach allows us to avoid any custom handling of units in the annotation processor, which means that it will generalize to any future metadata we would like to include. If it turns out that the regular expressions would be a significant quality of life improvement, we can consider that a separate feature.

How is custom metadata serialized

In Java, there is now a new kind of ValueSchema: a MetaSchema, which carries a Map<String, SerializedValue>, as well as a nested ValueSchema. The interpretation is that all of the metadata in the MetaSchema applies to the nested ValueSchema.

The withMeta method, used to construct a MetaSchema, checks to see if the inner value is itself a MetaSchema, and if so merges the two, overlaying the outer one on top of the inner one.

When serializing to json, this Map<String, SerializedValue> is attached to the nested object, like so:

MetaSchema(Map.of("unit", "m"), IntSchema()) becomes

{
  "type": "int",
  "metadata": {
    "unit": "m"
  }
}

Future-proofing

One of the goals of this PR is to lay the groundwork for adding additional metadata to value schemas.

Part of this PR brings annotations into the Resolver and TypePattern machinery. This allows us to generate value mappers for annotated types.

This PR allows users to define their own annotations, and as long as a value mapper is provided for that annotation, Aerie will be able to serialize it and include it in the value schema.

One generalization that this PR leaves undone is to allow for using value mapper methods to define value mappers for annotated types. The nuance here is that, in this PR, we have users use the @WithMetadata(Unit.class) annotation to declare an annotation of interest, and we use that to generate a type rule, rather than the users writing the type rule as the return type from a static method, the way that other value mappers are defined. This is due to a limitation of type use annotations that we may be able to work around in the future.

Verification

The Banananation example model was updated to include some unit information for parameters, computed attributes, and resources.

Things to test manually:

  • Do the value schemas look correct in the database?
  • Do scheduling and constraint checking still work with banananation? (I.e. do they correctly parse the new value schemas during typescript code generation)

The second bullet may be handled by e2e tests

Documentation

Future work

  • Incorporate unit information in typescript code generation
  • Provide a convenience @AutoValueMapper.Annotation to make adding your own metadata annotations less verbose
  • Find a workaround that allows type use annotations to be used for value mapper methods
  • // int[] repeated(); // TODO add support for arrays in annotations for which we generate value mappers

@mattdailis mattdailis requested a review from a team as a code owner October 5, 2023 18:58
@Mythicaeda Mythicaeda linked an issue Oct 5, 2023 that may be closed by this pull request
@mattdailis mattdailis force-pushed the user-metadata-annotations branch from 706142f to 416b628 Compare October 12, 2023 16:40
@mattdailis mattdailis temporarily deployed to e2e-test October 12, 2023 16:40 — with GitHub Actions Inactive
@mattdailis
Copy link
Collaborator Author

Fixed a broken test - valueSchemaIsNumeric was returning false for the /fruit resource, because it's wrapped in a meta schema. I added the case below:

private static boolean valueSchemaIsNumeric(final ValueSchema valueSchema) {
return valueSchema.equals(ValueSchema.INT)
|| valueSchema.equals(ValueSchema.REAL)
|| valueSchema.equals(LINEAR_RESOURCE_SCHEMA)
|| valueSchema.asMeta().map($ -> valueSchemaIsNumeric($.target())).orElse(false);
}

@mattdailis mattdailis force-pushed the user-metadata-annotations branch from 416b628 to 41fa6ad Compare October 17, 2023 18:37
@mattdailis mattdailis force-pushed the user-metadata-annotations branch from 41fa6ad to 7167780 Compare October 18, 2023 23:43
@mattdailis mattdailis temporarily deployed to e2e-test October 18, 2023 23:43 — with GitHub Actions Inactive
@mattdailis mattdailis force-pushed the user-metadata-annotations branch from 7167780 to 1aa1832 Compare October 19, 2023 02:15
@mattdailis mattdailis temporarily deployed to e2e-test October 19, 2023 02:15 — with GitHub Actions Inactive
@mattdailis mattdailis force-pushed the user-metadata-annotations branch from 1aa1832 to 1c8f031 Compare October 19, 2023 15:44
@mattdailis mattdailis temporarily deployed to e2e-test October 19, 2023 15:45 — with GitHub Actions Inactive
@mattdailis
Copy link
Collaborator Author

New e2e tests should be passing now. Updates since the walkthrough:

  • There is now an @AutoValueMapper.Annotation that can be used to generate a value mapper for a given annotation. It will generate a struct with the same keys as the annotation
    • We still need our own definition in BasicValueMappers, because our annotation processor doesn't apply to contrib. Perhaps a future work question of how we leverage our annotation processor in library code
  • I looked into separating TypePattern from ConcreteType, but ran into difficulties deciding which of the two should be returned from substitute. I think keeping them together is prudent
  • ParameterTest activity has been updated to use different units for the key and value of doubleMap, to allow us to ascertain that the annotations are applied in the right places
  • I added :examples:banananation:assemble as a dependency to the e2e-tests, so that it always uploads the most up-to-date version

@mattdailis mattdailis force-pushed the user-metadata-annotations branch from 1c8f031 to 2d79c22 Compare October 23, 2023 15:55
@mattdailis mattdailis temporarily deployed to e2e-test October 23, 2023 15:55 — with GitHub Actions Inactive
@mattdailis mattdailis force-pushed the user-metadata-annotations branch from 2d79c22 to a697d44 Compare October 23, 2023 21:38
@mattdailis mattdailis temporarily deployed to e2e-test October 23, 2023 21:38 — with GitHub Actions Inactive
@mattdailis
Copy link
Collaborator Author

For ease of review, I've pushed all changes as fixup commits (and there's one revert commit) - no changes have been made to earlier commits.

I looked into whether .box() on arrays should return this, or new ArrayPattern(this.element.box()), and determined that actually List<int[]> is a valid type 😲. I also took a look at TypeName's box() definition, and it returns this in all cases except for primitives.

@mattdailis mattdailis temporarily deployed to e2e-test October 26, 2023 16:39 — with GitHub Actions Inactive
@mattdailis mattdailis temporarily deployed to e2e-test October 26, 2023 16:50 — with GitHub Actions Inactive
@mattdailis mattdailis force-pushed the user-metadata-annotations branch from d1ff473 to 387f492 Compare October 27, 2023 23:39
@mattdailis mattdailis temporarily deployed to e2e-test October 31, 2023 23:50 — with GitHub Actions Inactive
@mattdailis mattdailis force-pushed the user-metadata-annotations branch from b087461 to e3368f2 Compare November 1, 2023 18:08
@mattdailis mattdailis temporarily deployed to e2e-test November 1, 2023 18:08 — with GitHub Actions Inactive
@mattdailis mattdailis force-pushed the user-metadata-annotations branch from e3368f2 to 62b7566 Compare November 1, 2023 22:16
@mattdailis mattdailis temporarily deployed to e2e-test November 1, 2023 22:16 — with GitHub Actions Inactive
@mattdailis mattdailis merged commit db90d42 into develop Nov 1, 2023
@mattdailis mattdailis deleted the user-metadata-annotations branch November 1, 2023 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pull Units from mission model annotations and store them in DB
2 participants