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

Add @AllChildren annotation #1367

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

adrienmaillard
Copy link
Contributor

@adrienmaillard adrienmaillard commented Mar 18, 2024

Description

Add a @AllChildren annotation to mission model so mission modelers can report, for a given activity, what other activity is called/spawned. This information will be used by the scheduler to evaluate whether it is useful to resimulate or not before goal evaluation: let's have G goal being satisfied by an activity of type A, G is the next goal to be evaluated. If an activity of type B was inserted at the last iteration, has B any chance of generating A and thus of satisfying the goal G ? If yes, simulation is required, otherwise it's not (for that only purpose, there might be other things that warrant a simulation, e.g. the goal needs to also evaluate an expression...).

Some edge cases:

  • If the user does not use the annotation for an activity, we make the worst case assumption that the activity might generate all activities (itself included).
  • Activities without effect model cannot generate children

This PR does not contain changed to the scheduler to use this information. These changes would overlap too much with changes under review in #1323

Verification

A test with the banananation model has been added to verify that the generated code is accurate.

Documentation

To be done.

Future work

Other things in #1364 + use the annotation in the scheduler.

children = activityTypeRecord.effectModel().get().children().get();
} else {
//if children have not been declared with an annotation, assume the activity might generate all activity types
children = missionModel.activityTypes().stream().map(ActivityTypeRecord::name).toArray(String[]::new);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory-wise, rather than returning a list of every activity in the plan, what about wrapping the return in Optional and returning Optional.empty? It would communicate the same idea-- that getChildren is returning the known children and that we don't know what the children are-- without needing a map of 300+ elements with each containing a list of 300+ elements (ie, in the Clipper case).

import java.util.Map;

public interface SchedulerModel {
Map<String, DurationType> getDurationTypes();
SerializedValue serializeDuration(final Duration duration);
Duration deserializeDuration(final SerializedValue serializedValue);
Map<String, List<String>> getChildren();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: If the data is provided in an array, what is gained by converting it into a List? Are there some List-only methods you expect the scheduler to use? Was it simpler to return a List in the code generator?

import java.util.Map;

public interface SchedulerModel {
Map<String, DurationType> getDurationTypes();
SerializedValue serializeDuration(final Duration duration);
Duration deserializeDuration(final SerializedValue serializedValue);
Map<String, List<String>> getChildren();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this change makes this PR breaking for existing models.

Comment on lines +336 to +346
.addStatement("final var result = new $T()", ParameterizedTypeName.get(ClassName.get(HashMap.class), ClassName.get(String.class), ParameterizedTypeName.get(List.class, String.class)))
.addCode(
missionModel
.activityTypes()
.stream()
.map(
activityTypeRecord ->
getChildrenStatement(missionModel, activityTypeRecord))
.reduce((x, y) -> x.add("$L", y.build()))
.orElse(CodeBlock.builder()).build())
.addStatement("return result")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, nonblocking: this method could return an immutable map if it uses the Map.ofEntries() method.
Then instead of each getChildrenStatement returning result.put("TYPE", LIST), it would return Map.entry("TYPE", LIST).

@adrienmaillard
Copy link
Contributor Author

This is blocked because part of an ongoing prototyping for dependency modeling in #1364.

@dandelany dandelany added the next label Apr 25, 2024
@adrienmaillard
Copy link
Contributor Author

@dandelany I think we can close this PR.

Copy link

@dandelany dandelany removed the next label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Blocked
Development

Successfully merging this pull request may close these issues.

3 participants