Fix plan duplication bug

NASA-AMMOS · Sep 5, 2024 · ee3b359 · ee3b359
1 parent 71ed927
commit ee3b359
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 46 deletions.
diff --git a/...st/java/gov/nasa/ammos/aerie/procedural/examples/fooprocedures/constraints/Procedure.java b/...st/java/gov/nasa/ammos/aerie/procedural/examples/fooprocedures/constraints/Procedure.java
diff --git a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java
@@ -15,14 +15,13 @@
 import gov.nasa.jpl.aerie.scheduler.simulation.SimulationFacade;
 import gov.nasa.jpl.aerie.scheduler.solver.ConflictSatisfaction;
 import gov.nasa.jpl.aerie.scheduler.solver.Evaluation;
-import org.apache.commons.lang3.NotImplementedException;
 
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Function;
 
-import static gov.nasa.jpl.aerie.scheduler.plan.InMemoryEditablePlan.toSchedulingActivityDirective;
+import static gov.nasa.jpl.aerie.scheduler.plan.InMemoryEditablePlan.toSchedulingActivity;
 
 public class Procedure extends Goal {
   private final Path jarPath;
@@ -58,24 +57,14 @@ public void run(Evaluation eval, Plan plan, MissionModel<?> missionModel, Functi
         lookupActivityType::apply
     );
 
-    /*
-     TODO
-
-     Comments from Joel:
-     - Part of the intent of editablePlan was to be able to re-use it across procedures.
-       - Could be done by initializing EditablePlanImpl with simulation results
-
-     Duration construction and arithmetic can be less awkward
-     */
-
     procedureMapper.deserialize(this.args).run(editablePlan);
 
     if (!editablePlan.getUncommittedChanges().isEmpty()) {
       throw new IllegalStateException("procedural goal %s had changes that were not committed or rolled back".formatted(jarPath.getFileName()));
     }
     for (final var edit : editablePlan.getTotalDiff()) {
       if (edit instanceof Edit.Create c) {
-        newActivities.add(toSchedulingActivityDirective(c.getDirective(), lookupActivityType::apply, true));
+        newActivities.add(toSchedulingActivity(c.getDirective(), lookupActivityType::apply, true));
       } else {
         throw new IllegalStateException("Unexpected value: " + edit);
       }

diff --git a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/model/Problem.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/model/Problem.java
@@ -134,7 +134,7 @@ public MissionModel<?> getMissionModel() {
    * @return the initial seed plan that schedulers may start from
    */
   public Plan getInitialPlan() {
-    return initialPlan;
+    return initialPlan.duplicate();
   }
 
   /**
@@ -148,7 +148,7 @@ public void setInitialPlan(
   ) {
     initialPlan = plan;
     this.initialSimulationResults = initialSimulationResults.map(simulationResults -> new SimulationData(
-        plan,
+        getInitialPlan(),
         simulationResults,
         SimulationResultsConverter.convertToConstraintModelResults(simulationResults)
     ));

diff --git a/...ver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/CheckpointSimulationFacade.java b/...ver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/CheckpointSimulationFacade.java
@@ -312,23 +312,8 @@ public SimulationData simulateWithResults(
   ) throws SimulationException, SchedulingInterruptedException {
     if (this.initialSimulationResults != null) {
       final var inputPlan = scheduleFromPlan(plan, schedulerModel);
-      final HashMap<ActivityDirectiveId, ActivityDirective> planActuallySimulated = new HashMap<>();
-      final var initialPlan = this.initialSimulationResults.plan().getActivitiesById();
-      var allActivitiesFound = true;
-      for (final var activityInstance: this.initialSimulationResults.constraintsResults().activities) {
-        if (activityInstance.directiveId().isPresent()) {
-          final var directiveId = activityInstance.directiveId().get();
-          final var directive = initialPlan.get(directiveId);
-          if (directive != null) {
-            planActuallySimulated.put(activityInstance.directiveId().get(), schedulingActToActivityDir(directive, schedulerModel));
-          } else {
-            allActivitiesFound = false;
-          }
-        }
-      }
-      if (allActivitiesFound && inputPlan.equals(new PlanSimCorrespondence(planActuallySimulated))) {
-        return initialSimulationResults;
-      }
+      final var initialPlan = scheduleFromPlan(this.initialSimulationResults.plan(), schedulerModel);
+      if (inputPlan.equals(initialPlan)) return initialSimulationResults;
     }
     final var resultsInput = simulateNoResults(plan, until);
     final var driverResults = resultsInput.simulationResultsComputerInputs().computeResults(resourceNames);

diff --git a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt
@@ -54,7 +54,7 @@ data class InMemoryEditablePlan(
     val resolved = directive.resolve(id, parent)
     uncommittedChanges.add(Edit.Create(resolved))
     resolved.validateArguments(lookupActivityType)
-    plan.add(resolved.toSchedulingActivityDirective(lookupActivityType, true))
+    plan.add(resolved.toSchedulingActivity(lookupActivityType, true))
     return id
   }
 
@@ -70,7 +70,7 @@ data class InMemoryEditablePlan(
     for (edit in result) {
       when (edit) {
         is Edit.Create -> {
-          plan.remove(edit.directive.toSchedulingActivityDirective(lookupActivityType, true))
+          plan.remove(edit.directive.toSchedulingActivity(lookupActivityType, true))
         }
       }
     }
@@ -93,7 +93,7 @@ data class InMemoryEditablePlan(
       lookupActivityType(type).specType.inputType.validateArguments(inner.arguments)
     }
 
-    @JvmStatic fun Directive<AnyDirective>.toSchedulingActivityDirective(lookupActivityType: (String) -> ActivityType, isNew: Boolean) = SchedulingActivity(
+    @JvmStatic fun Directive<AnyDirective>.toSchedulingActivity(lookupActivityType: (String) -> ActivityType, isNew: Boolean) = SchedulingActivity(
         id,
         lookupActivityType(type),
         when (val s = start) {

diff --git a/...iver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerToProcedurePlanAdapter.kt b/...iver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerToProcedurePlanAdapter.kt
@@ -1,19 +1,18 @@
 package gov.nasa.jpl.aerie.scheduler.plan
 
-import gov.nasa.jpl.aerie.merlin.protocol.types.Duration
-import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue
-import gov.nasa.jpl.aerie.scheduler.model.ActivityType
-import gov.nasa.jpl.aerie.scheduler.model.PlanningHorizon
 import gov.nasa.ammos.aerie.procedural.timeline.Interval
 import gov.nasa.ammos.aerie.procedural.timeline.collections.Directives
-import gov.nasa.ammos.aerie.procedural.timeline.util.duration.minus
-import gov.nasa.ammos.aerie.procedural.timeline.util.duration.plus
 import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.Directive
 import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.DirectiveStart
 import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.DirectiveStart.Anchor.AnchorPoint.Companion.anchorToStart
+import gov.nasa.ammos.aerie.procedural.timeline.util.duration.minus
+import gov.nasa.ammos.aerie.procedural.timeline.util.duration.plus
+import gov.nasa.jpl.aerie.merlin.protocol.types.Duration
+import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue
+import gov.nasa.jpl.aerie.scheduler.model.PlanningHorizon
 import java.time.Instant
-import gov.nasa.jpl.aerie.scheduler.model.Plan as SchedulerPlan
 import gov.nasa.ammos.aerie.procedural.timeline.plan.Plan as TimelinePlan
+import gov.nasa.jpl.aerie.scheduler.model.Plan as SchedulerPlan
 
 data class SchedulerToProcedurePlanAdapter(
     private val schedulerPlan: SchedulerPlan,