-
Notifications
You must be signed in to change notification settings - Fork 21
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
base: develop
Are you sure you want to change the base?
Conversation
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); |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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.
.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") |
There was a problem hiding this comment.
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)
.
This is blocked because part of an ongoing prototyping for dependency modeling in #1364. |
@dandelany I think we can close this PR. |
Quality Gate passedIssues Measures |
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:
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.