Merge pull request #1170 from NASA-AMMOS/1165-correct-scheduler-issue

Fix scheduler issue

e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/SchedulingTests.java

@@ -66,6 +66,18 @@ export default () => Goal.CoexistenceGoal({
         startsAt: TimingConstraint.singleton(WindowProperty.START)
       })""";
 
+  private final String bakeBananaGoalDefinition =
+      """
+      export default (): Goal =>
+          Goal.ActivityRecurrenceGoal({
+              activityTemplate: ActivityTemplates.BakeBananaBread({
+                  temperature: 325.0,
+                  tbSugar: 2,
+                  glutenFree: false,
+              }),
+              interval: Temporal.Duration.from({ hours: 12 }),
+          });""";
+
   @BeforeAll
   void beforeAll() {
     // Setup Requests
@@ -125,6 +137,30 @@ private void insertActivities() throws IOException {
     hasura.updatePlanRevisionSchedulingSpec(planId);
   }
 
+  //reproduces issue #1165
+  @Test
+  void twoInARow() throws IOException {
+    // Setup: Add Goal
+    final int bakeBananaBreadGoalId = hasura.insertSchedulingGoal(
+        "BakeBanana Scheduling Test Goal",
+        modelId,
+        bakeBananaGoalDefinition);
+    hasura.createSchedulingSpecGoal(bakeBananaBreadGoalId, schedulingSpecId, 0);
+    try {
+      // Schedule and get Plan
+      hasura.awaitScheduling(schedulingSpecId);
+      hasura.updatePlanRevisionSchedulingSpec(planId);
+      hasura.awaitScheduling(schedulingSpecId);
+      final var plan = hasura.getPlan(planId);
+      final var activities = plan.activityDirectives();
+      assertEquals(2, activities.size());
+      activities.forEach(a->assertEquals("BakeBananaBread", a.type()));
+    } finally {
+      // Teardown: Delete Goal
+      hasura.deleteSchedulingGoal(bakeBananaBreadGoalId);
+    }
+  }
+
   @Test
   void getSchedulingDSLTypeScript() throws IOException {
     final var schedulingDslTypes = hasura.getSchedulingDslTypeScript(modelId);

scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/model/Problem.java

@@ -141,8 +141,7 @@ public void setInitialPlan(final Plan plan, final Optional<SimulationResults> in
     this.initialSimulationResults = initialSimulationResults.map(simulationResults -> new SimulationData(
         simulationResults,
         SimulationResultsConverter.convertToConstraintModelResults(
-            simulationResults),
-        plan.getActivities()));
+            simulationResults)));
   }
 
   /**

scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/SimulationData.java

@@ -7,5 +7,5 @@
 
 public record SimulationData(
     SimulationResults driverResults,
-    gov.nasa.jpl.aerie.constraints.model.SimulationResults constraintsResults,
-    Collection<SchedulingActivityDirective> activitiesInPlan){}
+    gov.nasa.jpl.aerie.constraints.model.SimulationResults constraintsResults
+){}

scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/SimulationFacade.java

@@ -9,7 +9,6 @@
 import gov.nasa.jpl.aerie.merlin.protocol.types.Duration;
 import gov.nasa.jpl.aerie.merlin.protocol.types.DurationType;
 import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue;
-import gov.nasa.jpl.aerie.scheduler.model.Problem;
 import gov.nasa.jpl.aerie.scheduler.model.SchedulingActivityDirective;
 import gov.nasa.jpl.aerie.scheduler.model.ActivityType;
 import gov.nasa.jpl.aerie.scheduler.model.PlanningHorizon;
@@ -82,15 +81,23 @@ public void initialSimulationResultsAreStale(){
     this.initialPlanHasBeenModified = true;
   }
 
+  /**
+   * @return true if initial simulation results are stale, false otherwise
+   */
+  public boolean areInitialSimulationResultsStale(){
+    return this.initialPlanHasBeenModified;
+  }
+
   public Optional<gov.nasa.jpl.aerie.constraints.model.SimulationResults> getLatestConstraintSimulationResults(){
     if(!initialPlanHasBeenModified && initialSimulationResults.isPresent()) return Optional.of(this.initialSimulationResults.get().constraintsResults());
     if(lastSimulationData == null) return Optional.empty();
     return Optional.of(lastSimulationData.constraintsResults());
   }
 
-  public SimulationResults getLatestDriverSimulationResults(){
-    if(!initialPlanHasBeenModified && initialSimulationResults.isPresent()) return this.initialSimulationResults.get().driverResults();
-    return lastSimulationData.driverResults();
+  public Optional<SimulationResults> getLatestDriverSimulationResults(){
+    if(!initialPlanHasBeenModified && initialSimulationResults.isPresent()) return Optional.of(this.initialSimulationResults.get().driverResults());
+    if(lastSimulationData == null) return Optional.empty();
+    return Optional.of(lastSimulationData.driverResults());
   }
 
   public SimulationFacade(final PlanningHorizon planningHorizon, final MissionModel<?> missionModel) {
@@ -157,10 +164,16 @@ private ActivityDirectiveId getIdOfRootParent(SimulationResults results, Simulat
   public Map<SchedulingActivityDirective, SchedulingActivityDirectiveId> getAllChildActivities(final Duration endTime)
   throws SimulationException
   {
-    if(insertedActivities.size() == 0) return Map.of();
-    computeSimulationResultsUntil(endTime);
+    var latestSimulationData = this.getLatestDriverSimulationResults();
+    //if no initial sim results and no sim has been performed, perform a sim and get the sim results
+    if(latestSimulationData.isEmpty()){
+      //useful only if there are activities to simulate for this case of getting child activities
+      if(insertedActivities.size() == 0) return Map.of();
+      computeSimulationResultsUntil(endTime);
+      latestSimulationData = this.getLatestDriverSimulationResults();
+    }
     final Map<SchedulingActivityDirective, SchedulingActivityDirectiveId> childActivities = new HashMap<>();
-    this.lastSimulationData.driverResults().simulatedActivities.forEach( (activityInstanceId, activity) -> {
+    latestSimulationData.get().simulatedActivities.forEach( (activityInstanceId, activity) -> {
       if (activity.parentId() == null) return;
       final var rootParent = getIdOfRootParent(this.lastSimulationData.driverResults(), activityInstanceId);
       final var schedulingActId = planActDirectiveIdToSimulationActivityDirectiveId.entrySet().stream().filter(
@@ -275,10 +288,11 @@ private void simulateActivities(final Collection<SchedulingActivityDirective> ac
       insertedActivities.put(activity, activityDirective);
     }
     try {
-    driver.simulateActivities(directivesToSimulate);
+      driver.simulateActivities(directivesToSimulate);
     } catch(Exception e){
       throw new SimulationException("An exception happened during simulation", e);
     }
+    this.lastSimulationData = null;
   }
 
   public static class SimulationException extends Exception {
@@ -298,7 +312,7 @@ public void computeSimulationResultsUntil(final Duration endTime) throws Simulat
       //compare references
       if(lastSimulationData == null || results != lastSimulationData.driverResults()) {
         //simulation results from the last simulation, as converted for use by the constraint evaluation engine
-        this.lastSimulationData = new SimulationData(results, SimulationResultsConverter.convertToConstraintModelResults(results), this.insertedActivities.keySet());
+        this.lastSimulationData = new SimulationData(results, SimulationResultsConverter.convertToConstraintModelResults(results));
       }
     } catch (Exception e){
       throw new SimulationException("An exception happened during simulation", e);

scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/solver/PrioritySolver.java

@@ -20,6 +20,7 @@
 import gov.nasa.jpl.aerie.scheduler.goals.OptionGoal;
 import gov.nasa.jpl.aerie.scheduler.model.*;
 import gov.nasa.jpl.aerie.scheduler.model.SchedulingActivityDirective;
+import gov.nasa.jpl.aerie.scheduler.simulation.SimulationData;
 import gov.nasa.jpl.aerie.scheduler.simulation.SimulationFacade;
 import org.apache.commons.lang3.tuple.Pair;
 import org.slf4j.Logger;
@@ -188,14 +189,7 @@ private InsertActivityResult checkAndInsertActs(Collection<SchedulingActivityDir
         plan.add(act);
         finalSetOfActsInserted.add(act);
       }
-      final Map<SchedulingActivityDirective, SchedulingActivityDirectiveId> allGeneratedActivities;
-      try {
-        allGeneratedActivities = simulationFacade.getAllChildActivities(simulationFacade.getCurrentSimulationEndTime());
-      } catch (SimulationFacade.SimulationException e) {
-        throw new RuntimeException("Exception while simulating to get child activities", e);
-      }
-      processNewGeneratedActivities(allGeneratedActivities);
-      final var replaced = pullActivityDurationsIfNecessary();
+      final var replaced = synchronizeSimulationWithSchedulerPlan();
       for(final var actReplaced : replaced.entrySet()){
         if(finalSetOfActsInserted.contains(actReplaced.getKey())){
           finalSetOfActsInserted.remove(actReplaced.getKey());
@@ -213,6 +207,19 @@ private InsertActivityResult checkAndInsertActs(Collection<SchedulingActivityDir
     return new InsertActivityResult(allGood, finalSetOfActsInserted);
   }
 
+  private Map<SchedulingActivityDirective, SchedulingActivityDirective> synchronizeSimulationWithSchedulerPlan() {
+    final Map<SchedulingActivityDirective, SchedulingActivityDirective> replacedInPlan;
+    try {
+      final var allGeneratedActivities =
+          simulationFacade.getAllChildActivities(simulationFacade.getCurrentSimulationEndTime());
+      processNewGeneratedActivities(allGeneratedActivities);
+      replacedInPlan = pullActivityDurationsIfNecessary();
+    } catch (SimulationFacade.SimulationException e) {
+      throw new RuntimeException("Exception while simulating to get child activities", e);
+    }
+    return replacedInPlan;
+  }
+
   /**
    * creates internal storage space to build up partial solutions in
    **/
@@ -786,9 +793,7 @@ private SimulationResults getLatestSimResultsUpTo(Duration time){
     if (lastSimResultsFromFacade.isEmpty() || lastSimResultsFromFacade.get().bounds.end.shorterThan(time)) {
       try {
         this.simulationFacade.computeSimulationResultsUntil(time);
-        final var allGeneratedActivities = simulationFacade.getAllChildActivities(time);
-        processNewGeneratedActivities(allGeneratedActivities);
-        pullActivityDurationsIfNecessary();
+        synchronizeSimulationWithSchedulerPlan();
       } catch (SimulationFacade.SimulationException e) {
         throw new RuntimeException("Exception while running simulation before evaluating conflicts", e);
       }

scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/PrioritySolverTest.java

@@ -262,7 +262,7 @@ public void getNextSolution_coexistenceGoalOnActivityWorks_withInitialSimResults
     final var adHocFacade = new SimulationFacade(problem.getPlanningHorizon(), problem.getMissionModel());
     adHocFacade.insertActivitiesIntoSimulation(makePlanA012(problem).getActivities());
     adHocFacade.computeSimulationResultsUntil(problem.getPlanningHorizon().getEndAerie());
-    final var simResults = adHocFacade.getLatestDriverSimulationResults();
+    final var simResults = adHocFacade.getLatestDriverSimulationResults().get();
     problem.setInitialPlan(makePlanA012(problem), Optional.of(simResults));
 
     final var actTypeA = problem.getActivityType("ControllableDurationActivity");

scheduler-worker/src/main/java/gov/nasa/jpl/aerie/scheduler/worker/services/SynchronousSchedulerAgent.java

@@ -132,7 +132,7 @@ public void schedule(final ScheduleRequest request, final ResultsProtocol.Writer
         final var externalProfiles = loadExternalProfiles(planMetadata.planId());
         final var initialSimulationResults = loadSimulationResults(planMetadata);
         //seed the problem with the initial plan contents
-        final var loadedPlanComponents = loadInitialPlan(planMetadata, problem);
+        final var loadedPlanComponents = loadInitialPlan(planMetadata, problem, initialSimulationResults);
         problem.setInitialPlan(loadedPlanComponents.schedulerPlan(), initialSimulationResults);
         problem.setExternalProfile(externalProfiles.realProfiles(), externalProfiles.discreteProfiles());
         //apply constraints/goals to the problem
@@ -291,6 +291,7 @@ private Optional<DatasetId> storeSimulationResults(PlanningHorizon planningHoriz
                                                      final Map<SchedulingActivityDirective, ActivityDirectiveId> schedDirectiveToMerlinId)
   throws MerlinServiceException, IOException
   {
+    if(!simulationFacade.areInitialSimulationResultsStale()) return Optional.empty();
     //finish simulation until end of horizon before posting results
     try {
       simulationFacade.computeSimulationResultsUntil(planningHorizon.getEndAerie());
@@ -309,7 +310,7 @@ private Optional<DatasetId> storeSimulationResults(PlanningHorizon planningHoriz
             (a) -> schedID_to_MerlinID.get(a.getKey())));
     if(simID_to_MerlinID.values().containsAll(schedDirectiveToMerlinId.values()) && schedDirectiveToMerlinId.values().containsAll(simID_to_MerlinID.values())){
       return Optional.of(merlinService.storeSimulationResults(planMetadata,
-                                                              simulationFacade.getLatestDriverSimulationResults(),
+                                                              simulationFacade.getLatestDriverSimulationResults().get(),
                                                               simID_to_MerlinID));
     } else{
       //schedule in simulation is inconsistent with current state of the plan (user probably disabled simulation for some of the goals)
@@ -400,11 +401,15 @@ private Solver createScheduler(final PlanMetadata planMetadata, final Problem pr
    *
    * @param planMetadata metadata of plan container to load from
    * @param problem the problem that the plan adheres to
+   * @param initialSimulationResults initial simulation results (optional)
    * @return a plan with all activity instances loaded from the target merlin plan container
    * @throws ResultsProtocolFailure when the requested plan cannot be loaded, or the target plan revision has
    *     changed, or aerie could not be reached
    */
-  private PlanComponents loadInitialPlan(final PlanMetadata planMetadata, final Problem problem) {
+  private PlanComponents loadInitialPlan(
+      final PlanMetadata planMetadata,
+      final Problem problem,
+      final Optional<SimulationResults> initialSimulationResults) {
     //TODO: maybe paranoid check if plan rev has changed since original metadata?
     try {
       final var merlinPlan =  merlinService.getPlanActivityDirectives(planMetadata, problem);
@@ -430,12 +435,19 @@ private PlanComponents loadInitialPlan(final PlanMetadata planMetadata, final Pr
                     .orElseThrow(() -> new Exception("Controllable Duration parameter was not an Int")),
                 Duration.MICROSECONDS);
           }
-        } else if (
-            schedulerActType.getDurationType() instanceof DurationType.Uncontrollable
-            || schedulerActType.getDurationType() instanceof DurationType.Fixed
-            || schedulerActType.getDurationType() instanceof DurationType.Parametric
-        ) {
-          // Do nothing
+        } else if (schedulerActType.getDurationType() instanceof DurationType.Fixed fixedDurationType) {
+          actDuration = fixedDurationType.duration();
+        } else if(schedulerActType.getDurationType() instanceof DurationType.Parametric parametricDurationType) {
+          actDuration = parametricDurationType.durationFunction().apply(activity.serializedActivity().getArguments());
+        } else if(schedulerActType.getDurationType() instanceof DurationType.Uncontrollable) {
+          if(initialSimulationResults.isPresent()){
+            for(final var simAct: initialSimulationResults.get().simulatedActivities.entrySet()){
+              if(simAct.getValue().directiveId().isPresent() &&
+                 simAct.getValue().directiveId().get().equals(elem.getKey())){
+                actDuration = simAct.getValue().duration();
+              }
+            }
+          }
         } else {
           throw new Error("Unhandled variant of DurationType:" + schedulerActType.getDurationType());
         }