Fix unplanning bug for plan one of plan unit

model_plan_units_unit_test.go

@@ -237,6 +237,18 @@ func TestModel_NewPlanOneOfPlanUnits(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	if solution.UnPlannedPlanUnits().Size() != 1 {
+		t.Fatal("solution should have 1 un-planned plan units, it has", solution.UnPlannedPlanUnits().Size())
+	}
+
+	if solution.SolutionPlanUnit(dpu1ordpu2) != solution.UnPlannedPlanUnits().SolutionPlanUnits()[0] {
+		t.Fatal("The top-level plan unit should be unplanned")
+	}
+
+	if solution.PlannedPlanUnits().Size() != 0 {
+		t.Fatal("solution should have 0 planned plan units, it has", solution.PlannedPlanUnits().Size())
+	}
+
 	move := solution.BestMove(context.Background(), solution.SolutionPlanUnit(dpu1ordpu2))
 
 	if !move.IsExecutable() {
@@ -251,9 +263,78 @@ func TestModel_NewPlanOneOfPlanUnits(t *testing.T) {
 	for _, stop := range solution.Vehicles()[0].SolutionStops() {
 		fmt.Println(stop.ModelStop().ID())
 	}
+
+	if solution.UnPlannedPlanUnits().Size() != 0 {
+		t.Fatal("solution should have 0 un-planned plan units, it has", solution.UnPlannedPlanUnits().Size())
+	}
+
+	if solution.PlannedPlanUnits().Size() != 1 {
+		t.Fatal("solution should have 1 planned plan units, it has", solution.PlannedPlanUnits().Size())
+	}
+
+	if solution.SolutionPlanUnit(dpu1ordpu2) != solution.PlannedPlanUnits().SolutionPlanUnits()[0] {
+		t.Fatal("The top-level plan unit should be planned")
+	}
+
+	solutionPlanUnit := solution.SolutionPlanUnit(dpu1ordpu2) 
+	unplanned, err := solutionPlanUnit.UnPlan()
+
+	if !unplanned {
+		t.Fatal("Could not unplan OneOfPlanUnit")
+	}
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if solutionPlanUnit.IsPlanned() {
+		t.Fatal("Could not unplan OneOfPlanUnit")
+	}
+
+	if solution.UnPlannedPlanUnits().Size() != 1 {
+		t.Fatal("solution should have 1 un-planned plan units, it has", solution.UnPlannedPlanUnits().Size())
+	}
+
+	if solutionPlanUnit != solution.UnPlannedPlanUnits().SolutionPlanUnits()[0] {
+		t.Fatal("The top-level plan unit should be unplanned")
+	}
+
+	if solution.PlannedPlanUnits().Size() != 0 {
+		t.Fatal("solution should have 0 planned plan units, it has", solution.PlannedPlanUnits().Size())
+	}
+
+	for _, stop := range solution.Vehicles()[0].SolutionStops() {
+		fmt.Println(stop.ModelStop().ID())
+	}
 }
 
 func TestPlanUnitsUnit(t *testing.T) {
+	testCases := []struct {
+		name                string
+		createPlanUnitsUnit func(model nextroute.Model, s1Unit, s2Unit nextroute.ModelPlanUnit) (nextroute.ModelPlanUnitsUnit, error)
+	}{
+		{
+			name: "PlanOneOfPlanUnits",
+			createPlanUnitsUnit: func(model nextroute.Model, s1Unit, s2Unit nextroute.ModelPlanUnit) (nextroute.ModelPlanUnitsUnit, error) {
+				return model.NewPlanOneOfPlanUnits(s1Unit, s2Unit)
+			},
+		},
+		{
+			name: "PlanAllPlanUnits",
+			createPlanUnitsUnit: func(model nextroute.Model, s1Unit, s2Unit nextroute.ModelPlanUnit) (nextroute.ModelPlanUnitsUnit, error) {
+				return model.NewPlanAllPlanUnits(true, s1Unit, s2Unit)
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			testPlanUnitsUnit(t, tc.createPlanUnitsUnit)
+		})
+	}
+}
+
+func testPlanUnitsUnit(t *testing.T, createPlanUnitsUnit func(model nextroute.Model, s1Unit, s2Unit nextroute.ModelPlanUnit) (nextroute.ModelPlanUnitsUnit, error)) {
 	model, err := nextroute.NewModel()
 	if err != nil {
 		t.Fatal(err)
@@ -294,7 +375,7 @@ func TestPlanUnitsUnit(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	s1ors2Unit, err := model.NewPlanAllPlanUnits(true, s1Unit, s2Unit)
+	s1ors2Unit, err := createPlanUnitsUnit(model, s1Unit, s2Unit)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -355,6 +436,15 @@ func TestPlanUnitsUnit(t *testing.T) {
 	if !success {
 		t.Fatal("move should be successful")
 	}
+
+	if solution.PlannedPlanUnits().Size() != 1 {
+		t.Fatal("solution should have 1 planned plan units, it has", solution.PlannedPlanUnits().Size())
+	}
+
+	if solution.UnPlannedPlanUnits().Size() != 1 {
+		t.Fatal("solution should have 1 un-planned plan units, it has", solution.UnPlannedPlanUnits().Size())
+	}
+
 	move = solution.BestMove(
 		context.Background(),
 		solution.UnPlannedPlanUnits().SolutionPlanUnit(s3Unit),
@@ -375,6 +465,14 @@ func TestPlanUnitsUnit(t *testing.T) {
 		t.Fatal("move should be successful")
 	}
 
+	if solution.PlannedPlanUnits().Size() != 2 {
+		t.Fatal("solution should have 2 planned plan units, it has", solution.PlannedPlanUnits().Size())
+	}
+
+	if solution.UnPlannedPlanUnits().Size() != 0 {
+		t.Fatal("solution should have 0 un-planned plan units, it has", solution.UnPlannedPlanUnits().Size())
+	}
+
 	/*
 		for _, v := range solution.Vehicles() {
 			fmt.Println("-")

solution_move_units.go

@@ -11,15 +11,6 @@ func newSolutionMoveUnits(
 	planUnit *solutionPlanUnitsUnitImpl,
 	moves SolutionMoves,
 ) solutionMoveUnitsImpl {
-	if len(moves) != len(planUnit.solutionPlanUnits) {
-		panic(
-			fmt.Sprintf("moves and SolutionPlanUnits must have the same length: %v != %v",
-				len(moves),
-				len(planUnit.solutionPlanUnits),
-			),
-		)
-	}
-
 	value := 0.0
 	for _, move := range moves {
 		value += move.Value()
@@ -68,8 +59,11 @@ func (m solutionMoveUnitsImpl) Execute(ctx context.Context) (bool, error) {
 
 	m.solution.model.OnPlan(m)
 
-	m.solution.unPlannedPlanUnits.remove(m.planUnit)
-	m.solution.plannedPlanUnits.add(m.planUnit)
+	// Guarding against nested plan units unit. Only the top-level plan unit should be part of the accounting
+	if _, isElementOfPlanUnitsUnit := m.planUnit.ModelPlanUnit().PlanUnitsUnit(); !isElementOfPlanUnitsUnit {
+		m.solution.unPlannedPlanUnits.remove(m.planUnit)
+		m.solution.plannedPlanUnits.add(m.planUnit)
+	}
 
 	for idx, move := range m.moves {
 		if planned, err := move.Execute(ctx); err != nil || !planned {

solution_plan_stops_unit.go

@@ -134,11 +134,7 @@ func (p *solutionPlanStopsUnitImpl) UnPlan() (bool, error) {
 
 	solution.Model().OnUnPlan(p)
 
-	if planUnitsUnit, isMemberOf := p.modelPlanStopsUnit.PlanUnitsUnit(); isMemberOf {
-		solutionPlanUnitsUnit := solution.SolutionPlanUnit(planUnitsUnit)
-		solution.plannedPlanUnits.remove(solutionPlanUnitsUnit)
-		solution.unPlannedPlanUnits.add(solutionPlanUnitsUnit)
-	} else {
+	if _, isMemberOf := p.modelPlanStopsUnit.PlanUnitsUnit(); !isMemberOf {
 		solution.plannedPlanUnits.remove(p)
 		solution.unPlannedPlanUnits.add(p)
 	}

solution_plan_units_unit.go

@@ -129,9 +129,11 @@ func (p *solutionPlanUnitsUnitImpl) UnPlan() (bool, error) {
 	}
 
 	solution := p.Solution().(*solutionImpl)
-
-	solution.plannedPlanUnits.remove(p)
-	solution.unPlannedPlanUnits.add(p)
+	// Guard against nested plan units unit. Only the top-level plan unit should be accounted for.
+	if _, isMemberOf := p.modelPlanUnitsUnit.PlanUnitsUnit(); !isMemberOf {
+		solution.plannedPlanUnits.remove(p)
+		solution.unPlannedPlanUnits.add(p)
+	}
 
 	for _, solutionPlanUnit := range p.solutionPlanUnits {
 		if solutionPlanUnit.IsPlanned() {

solution_vehicle.go

@@ -353,12 +353,15 @@ func (v SolutionVehicle) bestMovePlanOneOfUnit(
 ) SolutionMove {
 	move := NotExecutableMove
 
-	for _, planUnit := range planUnit.solutionPlanUnits {
+	for _, pu := range planUnit.solutionPlanUnits {
 		move = move.TakeBest(
-			v.BestMove(ctx, planUnit),
+			v.BestMove(ctx, pu),
 		)
 	}
-	return move
+
+	// This returns a SolutionMoveUnits wrapper around the move. Otherwise the move cannot remove
+	// the plan unit from the solution when move.Execute() is called.
+	return newSolutionMoveUnits(planUnit, SolutionMoves{move})
 }
 
 func revertMoves(moves SolutionMoves) (bool, error) {