From ee3b359f41c581cfd20d70c0ac1c63c4ab36ae45 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 5 Sep 2024 14:48:15 -0700 Subject: [PATCH] Fix plan duplication bug --- .../fooprocedures/constraints/Procedure.java | 4 ---- .../jpl/aerie/scheduler/goals/Procedure.java | 15 ++------------- .../jpl/aerie/scheduler/model/Problem.java | 4 ++-- .../CheckpointSimulationFacade.java | 19 ++----------------- .../scheduler/plan/InMemoryEditablePlan.kt | 6 +++--- .../plan/SchedulerToProcedurePlanAdapter.kt | 13 ++++++------- 6 files changed, 15 insertions(+), 46 deletions(-) delete mode 100644 procedural/examples/foo-procedures/src/test/java/gov/nasa/ammos/aerie/procedural/examples/fooprocedures/constraints/Procedure.java diff --git a/procedural/examples/foo-procedures/src/test/java/gov/nasa/ammos/aerie/procedural/examples/fooprocedures/constraints/Procedure.java b/procedural/examples/foo-procedures/src/test/java/gov/nasa/ammos/aerie/procedural/examples/fooprocedures/constraints/Procedure.java deleted file mode 100644 index cd1cd5927d..0000000000 --- a/procedural/examples/foo-procedures/src/test/java/gov/nasa/ammos/aerie/procedural/examples/fooprocedures/constraints/Procedure.java +++ /dev/null @@ -1,4 +0,0 @@ -package gov.nasa.ammos.aerie.procedural.examples.fooprocedures.constraints; - -public interface Procedure { -} 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 index 90c3ca9f9c..a4ed23538d 100644 --- 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,16 +57,6 @@ 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()) { @@ -75,7 +64,7 @@ public void run(Evaluation eval, Plan plan, MissionModel missionModel, Functi } 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 index 1af4804706..a2ffc9e7f7 100644 --- 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/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/CheckpointSimulationFacade.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/CheckpointSimulationFacade.java index f5fb07efad..834bac8896 100644 --- a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/CheckpointSimulationFacade.java +++ b/scheduler-driver/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 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 index 26591b71c6..2652c03abf 100644 --- 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.toSchedulingActivityDirective(lookupActivityType: (String) -> ActivityType, isNew: Boolean) = SchedulingActivity( + @JvmStatic fun Directive.toSchedulingActivity(lookupActivityType: (String) -> ActivityType, isNew: Boolean) = SchedulingActivity( id, lookupActivityType(type), when (val s = start) { diff --git a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerToProcedurePlanAdapter.kt b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerToProcedurePlanAdapter.kt index b7c7b46674..71a1b78263 100644 --- a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerToProcedurePlanAdapter.kt +++ b/scheduler-driver/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,