fix: unimproved terminations skip construction heuristic (#755)

Co-authored-by: Frederico Gonçalves <[email protected]>
TimefoldAI · Mar 27, 2024 · a7e7a19 · a7e7a19
1 parent f1e8bd8
commit a7e7a19
Show file tree

Hide file tree

Showing 22 changed files with 464 additions and 207 deletions.
diff --git a/...imefold/solver/core/impl/constructionheuristic/scope/ConstructionHeuristicPhaseScope.java b/...imefold/solver/core/impl/constructionheuristic/scope/ConstructionHeuristicPhaseScope.java
@@ -12,7 +12,7 @@ public class ConstructionHeuristicPhaseScope<Solution_> extends AbstractPhaseSco
     private ConstructionHeuristicStepScope<Solution_> lastCompletedStepScope;
 
     public ConstructionHeuristicPhaseScope(SolverScope<Solution_> solverScope) {
-        super(solverScope);
+        super(solverScope, false);
         lastCompletedStepScope = new ConstructionHeuristicStepScope<>(this, -1);
     }
 

diff --git a/.../java/ai/timefold/solver/core/impl/exhaustivesearch/scope/ExhaustiveSearchPhaseScope.java b/.../java/ai/timefold/solver/core/impl/exhaustivesearch/scope/ExhaustiveSearchPhaseScope.java
@@ -22,7 +22,7 @@ public class ExhaustiveSearchPhaseScope<Solution_> extends AbstractPhaseScope<So
     private ExhaustiveSearchStepScope<Solution_> lastCompletedStepScope;
 
     public ExhaustiveSearchPhaseScope(SolverScope<Solution_> solverScope) {
-        super(solverScope);
+        super(solverScope, false);
         lastCompletedStepScope = new ExhaustiveSearchStepScope<>(this, -1);
     }
 

diff --git a/core/src/main/java/ai/timefold/solver/core/impl/phase/custom/scope/CustomPhaseScope.java b/core/src/main/java/ai/timefold/solver/core/impl/phase/custom/scope/CustomPhaseScope.java
@@ -12,7 +12,11 @@ public class CustomPhaseScope<Solution_> extends AbstractPhaseScope<Solution_> {
     private CustomStepScope<Solution_> lastCompletedStepScope;
 
     public CustomPhaseScope(SolverScope<Solution_> solverScope) {
-        super(solverScope);
+        this(solverScope, false);
+    }
+
+    public CustomPhaseScope(SolverScope<Solution_> solverScope, boolean phaseSendsBestSolutionEvents) {
+        super(solverScope, phaseSendsBestSolutionEvents);
         lastCompletedStepScope = new CustomStepScope<>(this, -1);
     }
 

diff --git a/core/src/main/java/ai/timefold/solver/core/impl/phase/scope/AbstractPhaseScope.java b/core/src/main/java/ai/timefold/solver/core/impl/phase/scope/AbstractPhaseScope.java
@@ -19,6 +19,7 @@ public abstract class AbstractPhaseScope<Solution_> {
     protected final transient Logger logger = LoggerFactory.getLogger(getClass());
 
     protected final SolverScope<Solution_> solverScope;
+    protected final boolean phaseSendingBestSolutionEvents;
 
     protected Long startingSystemTimeMillis;
     protected Long startingScoreCalculationCount;
@@ -29,14 +30,35 @@ public abstract class AbstractPhaseScope<Solution_> {
 
     protected int bestSolutionStepIndex;
 
-    public AbstractPhaseScope(SolverScope<Solution_> solverScope) {
+    /**
+     * As defined by #AbstractPhaseScope(SolverScope, boolean),
+     * with the phaseSendingBestSolutionEvents parameter set to true.
+     */
+    protected AbstractPhaseScope(SolverScope<Solution_> solverScope) {
+        this(solverScope, true);
+    }
+
+    /**
+     *
+     * @param solverScope never null
+     * @param phaseSendingBestSolutionEvents set to false if the phase only sends one best solution event at the end,
+     *        or none at all;
+     *        this is typical for construction heuristics,
+     *        whose result only matters when it reached its natural end.
+     */
+    protected AbstractPhaseScope(SolverScope<Solution_> solverScope, boolean phaseSendingBestSolutionEvents) {
         this.solverScope = solverScope;
+        this.phaseSendingBestSolutionEvents = phaseSendingBestSolutionEvents;
     }
 
     public SolverScope<Solution_> getSolverScope() {
         return solverScope;
     }
 
+    public boolean isPhaseSendingBestSolutionEvents() {
+        return phaseSendingBestSolutionEvents;
+    }
+
     public Long getStartingSystemTimeMillis() {
         return startingSystemTimeMillis;
     }

diff --git a/...in/java/ai/timefold/solver/core/impl/solver/termination/AbstractCompositeTermination.java b/...in/java/ai/timefold/solver/core/impl/solver/termination/AbstractCompositeTermination.java
@@ -15,7 +15,9 @@
  * @see AndCompositeTermination
  * @see OrCompositeTermination
  */
-public abstract class AbstractCompositeTermination<Solution_> extends AbstractTermination<Solution_> {
+public abstract sealed class AbstractCompositeTermination<Solution_>
+        extends AbstractTermination<Solution_>
+        permits AndCompositeTermination, OrCompositeTermination {
 
     protected final List<Termination<Solution_>> terminationList;
 

diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AbstractTermination.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AbstractTermination.java
@@ -7,23 +7,22 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/**
- * Abstract superclass for {@link Termination}.
- */
-public abstract class AbstractTermination<Solution_> extends PhaseLifecycleListenerAdapter<Solution_>
-        implements Termination<Solution_> {
+public abstract sealed class AbstractTermination<Solution_>
+        extends PhaseLifecycleListenerAdapter<Solution_>
+        implements Termination<Solution_>
+        permits AbstractCompositeTermination, BasicPlumbingTermination, BestScoreFeasibleTermination, BestScoreTermination,
+        ChildThreadPlumbingTermination, PhaseToSolverTerminationBridge, ScoreCalculationCountTermination, StepCountTermination,
+        TimeMillisSpentTermination, UnimprovedStepCountTermination,
+        UnimprovedTimeMillisSpentScoreDifferenceThresholdTermination, UnimprovedTimeMillisSpentTermination {
 
     protected final transient Logger logger = LoggerFactory.getLogger(getClass());
 
-    // ************************************************************************
-    // Other methods
-    // ************************************************************************
-
     @Override
     public Termination<Solution_> createChildThreadTermination(SolverScope<Solution_> solverScope,
             ChildThreadType childThreadType) {
-        throw new UnsupportedOperationException("This terminationClass (" + getClass()
-                + ") does not yet support being used in child threads of type (" + childThreadType + ").");
+        throw new UnsupportedOperationException(
+                "This terminationClass (%s) does not yet support being used in child threads of type (%s)."
+                        .formatted(getClass().getSimpleName(), childThreadType));
     }
 
 }
diff --git a/...rc/main/java/ai/timefold/solver/core/impl/solver/termination/AndCompositeTermination.java b/...rc/main/java/ai/timefold/solver/core/impl/solver/termination/AndCompositeTermination.java
@@ -6,7 +6,7 @@
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
 import ai.timefold.solver.core.impl.solver.thread.ChildThreadType;
 
-public class AndCompositeTermination<Solution_> extends AbstractCompositeTermination<Solution_> {
+public final class AndCompositeTermination<Solution_> extends AbstractCompositeTermination<Solution_> {
 
     public AndCompositeTermination(List<Termination<Solution_>> terminationList) {
         super(terminationList);

diff --git a/...c/main/java/ai/timefold/solver/core/impl/solver/termination/BasicPlumbingTermination.java b/...c/main/java/ai/timefold/solver/core/impl/solver/termination/BasicPlumbingTermination.java
@@ -14,15 +14,13 @@
  * Concurrency notes:
  * Condition predicate on ({@link #problemFactChangeQueue} is not empty or {@link #terminatedEarly} is true).
  */
-public class BasicPlumbingTermination<Solution_> extends AbstractTermination<Solution_> {
+public final class BasicPlumbingTermination<Solution_> extends AbstractTermination<Solution_> {
 
-    protected final boolean daemon;
+    private final boolean daemon;
+    private final BlockingQueue<ProblemChangeAdapter<Solution_>> problemFactChangeQueue = new LinkedBlockingQueue<>();
 
-    protected boolean terminatedEarly = false;
-
-    protected BlockingQueue<ProblemChangeAdapter<Solution_>> problemFactChangeQueue = new LinkedBlockingQueue<>();
-
-    protected boolean problemFactChangesBeingProcessed = false;
+    private boolean terminatedEarly = false;
+    private boolean problemFactChangesBeingProcessed = false;
 
     public BasicPlumbingTermination(boolean daemon) {
         this.daemon = daemon;

diff --git a/...in/java/ai/timefold/solver/core/impl/solver/termination/BestScoreFeasibleTermination.java b/...in/java/ai/timefold/solver/core/impl/solver/termination/BestScoreFeasibleTermination.java
@@ -8,20 +8,19 @@
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
 import ai.timefold.solver.core.impl.solver.thread.ChildThreadType;
 
-public class BestScoreFeasibleTermination<Solution_> extends AbstractTermination<Solution_> {
+public final class BestScoreFeasibleTermination<Solution_> extends AbstractTermination<Solution_> {
 
     private final int feasibleLevelsSize;
     private final double[] timeGradientWeightFeasibleNumbers;
 
-    public BestScoreFeasibleTermination(ScoreDefinition scoreDefinition, double[] timeGradientWeightFeasibleNumbers) {
-        feasibleLevelsSize = scoreDefinition.getFeasibleLevelsSize();
+    public BestScoreFeasibleTermination(ScoreDefinition<?> scoreDefinition, double[] timeGradientWeightFeasibleNumbers) {
+        this.feasibleLevelsSize = scoreDefinition.getFeasibleLevelsSize();
         this.timeGradientWeightFeasibleNumbers = timeGradientWeightFeasibleNumbers;
-        if (timeGradientWeightFeasibleNumbers.length != feasibleLevelsSize - 1) {
+        if (timeGradientWeightFeasibleNumbers.length != this.feasibleLevelsSize - 1) {
             throw new IllegalStateException(
-                    "The timeGradientWeightNumbers (" + Arrays.toString(timeGradientWeightFeasibleNumbers)
-                            + ")'s length (" + timeGradientWeightFeasibleNumbers.length
-                            + ") is not 1 less than the feasibleLevelsSize (" + scoreDefinition.getFeasibleLevelsSize()
-                            + ").");
+                    "The timeGradientWeightNumbers (%s)'s length (%d) is not 1 less than the feasibleLevelsSize (%d)."
+                            .formatted(Arrays.toString(timeGradientWeightFeasibleNumbers),
+                                    timeGradientWeightFeasibleNumbers.length, scoreDefinition.getFeasibleLevelsSize()));
         }
     }
 
@@ -32,10 +31,10 @@ public boolean isSolverTerminated(SolverScope<Solution_> solverScope) {
 
     @Override
     public boolean isPhaseTerminated(AbstractPhaseScope<Solution_> phaseScope) {
-        return isTerminated((Score) phaseScope.getBestScore());
+        return isTerminated(phaseScope.getBestScore());
     }
 
-    protected boolean isTerminated(Score bestScore) {
+    private static boolean isTerminated(Score<?> bestScore) {
         return bestScore.isFeasible();
     }
 
@@ -49,7 +48,7 @@ public double calculatePhaseTimeGradient(AbstractPhaseScope<Solution_> phaseScop
         return calculateFeasibilityTimeGradient((Score) phaseScope.getStartingScore(), (Score) phaseScope.getBestScore());
     }
 
-    protected <Score_ extends Score<Score_>> double calculateFeasibilityTimeGradient(Score_ startScore, Score_ score) {
+    <Score_ extends Score<Score_>> double calculateFeasibilityTimeGradient(Score_ startScore, Score_ score) {
         if (startScore == null || !startScore.isSolutionInitialized()) {
             return 0.0;
         }

diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/BestScoreTermination.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/BestScoreTermination.java
@@ -8,25 +8,25 @@
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
 import ai.timefold.solver.core.impl.solver.thread.ChildThreadType;
 
-public class BestScoreTermination<Solution_> extends AbstractTermination<Solution_> {
+public final class BestScoreTermination<Solution_> extends AbstractTermination<Solution_> {
 
     private final int levelsSize;
     private final Score bestScoreLimit;
     private final double[] timeGradientWeightNumbers;
 
-    public BestScoreTermination(ScoreDefinition scoreDefinition, Score bestScoreLimit, double[] timeGradientWeightNumbers) {
+    public BestScoreTermination(ScoreDefinition<?> scoreDefinition, Score<?> bestScoreLimit,
+            double[] timeGradientWeightNumbers) {
         levelsSize = scoreDefinition.getLevelsSize();
         this.bestScoreLimit = bestScoreLimit;
         if (bestScoreLimit == null) {
-            throw new IllegalArgumentException("The bestScoreLimit (" + bestScoreLimit
-                    + ") cannot be null.");
+            throw new IllegalArgumentException("The bestScoreLimit cannot be null.");
         }
         this.timeGradientWeightNumbers = timeGradientWeightNumbers;
         if (timeGradientWeightNumbers.length != levelsSize - 1) {
             throw new IllegalStateException(
-                    "The timeGradientWeightNumbers (" + Arrays.toString(timeGradientWeightNumbers)
-                            + ")'s length (" + timeGradientWeightNumbers.length
-                            + ") is not 1 less than the levelsSize (" + scoreDefinition.getLevelsSize() + ").");
+                    "The timeGradientWeightNumbers (%s)'s length (%d) is not 1 less than the levelsSize (%d)."
+                            .formatted(Arrays.toString(timeGradientWeightNumbers), timeGradientWeightNumbers.length,
+                                    scoreDefinition.getLevelsSize()));
         }
     }
 
@@ -44,7 +44,7 @@ public boolean isPhaseTerminated(AbstractPhaseScope<Solution_> phaseScope) {
         return isTerminated(phaseScope.isBestSolutionInitialized(), (Score) phaseScope.getBestScore());
     }
 
-    protected boolean isTerminated(boolean bestSolutionInitialized, Score bestScore) {
+    private boolean isTerminated(boolean bestSolutionInitialized, Score bestScore) {
         return bestSolutionInitialized && bestScore.compareTo(bestScoreLimit) >= 0;
     }
 
@@ -54,8 +54,8 @@ protected boolean isTerminated(boolean bestSolutionInitialized, Score bestScore)
 
     @Override
     public double calculateSolverTimeGradient(SolverScope<Solution_> solverScope) {
-        Score startingInitializedScore = solverScope.getStartingInitializedScore();
-        Score bestScore = solverScope.getBestScore();
+        var startingInitializedScore = solverScope.getStartingInitializedScore();
+        var bestScore = solverScope.getBestScore();
         return calculateTimeGradient(startingInitializedScore, bestScoreLimit, bestScore);
     }
 
@@ -66,12 +66,11 @@ public double calculatePhaseTimeGradient(AbstractPhaseScope<Solution_> phaseScop
         return calculateTimeGradient(startingInitializedScore, bestScoreLimit, bestScore);
     }
 
-    protected <Score_ extends Score<Score_>> double calculateTimeGradient(Score_ startScore, Score_ endScore,
-            Score_ score) {
-        Score_ totalDiff = endScore.subtract(startScore);
-        Number[] totalDiffNumbers = totalDiff.toLevelNumbers();
-        Score_ scoreDiff = score.subtract(startScore);
-        Number[] scoreDiffNumbers = scoreDiff.toLevelNumbers();
+    <Score_ extends Score<Score_>> double calculateTimeGradient(Score_ startScore, Score_ endScore, Score_ score) {
+        var totalDiff = endScore.subtract(startScore);
+        var totalDiffNumbers = totalDiff.toLevelNumbers();
+        var scoreDiff = score.subtract(startScore);
+        var scoreDiffNumbers = scoreDiff.toLevelNumbers();
         if (scoreDiffNumbers.length != totalDiffNumbers.length) {
             throw new IllegalStateException("The startScore (" + startScore + "), endScore (" + endScore
                     + ") and score (" + score + ") don't have the same levelsSize.");
@@ -90,9 +89,9 @@ protected <Score_ extends Score<Score_>> double calculateTimeGradient(Score_ sta
      */
     static double calculateTimeGradient(Number[] totalDiffNumbers, Number[] scoreDiffNumbers,
             double[] timeGradientWeightNumbers, int levelDepth) {
-        double timeGradient = 0.0;
-        double remainingTimeGradient = 1.0;
-        for (int i = 0; i < levelDepth; i++) {
+        var timeGradient = 0.0;
+        var remainingTimeGradient = 1.0;
+        for (var i = 0; i < levelDepth; i++) {
             double levelTimeGradientWeight;
             if (i != (levelDepth - 1)) {
                 levelTimeGradientWeight = remainingTimeGradient * timeGradientWeightNumbers[i];
@@ -101,8 +100,8 @@ static double calculateTimeGradient(Number[] totalDiffNumbers, Number[] scoreDif
                 levelTimeGradientWeight = remainingTimeGradient;
                 remainingTimeGradient = 0.0;
             }
-            double totalDiffLevel = totalDiffNumbers[i].doubleValue();
-            double scoreDiffLevel = scoreDiffNumbers[i].doubleValue();
+            var totalDiffLevel = totalDiffNumbers[i].doubleValue();
+            var scoreDiffLevel = scoreDiffNumbers[i].doubleValue();
             if (scoreDiffLevel == totalDiffLevel) {
                 // Max out this level
                 timeGradient += levelTimeGradientWeight;
@@ -118,7 +117,7 @@ static double calculateTimeGradient(Number[] totalDiffNumbers, Number[] scoreDif
                 // timeGradient += 0.0
                 break;
             } else {
-                double levelTimeGradient = scoreDiffLevel / totalDiffLevel;
+                var levelTimeGradient = scoreDiffLevel / totalDiffLevel;
                 timeGradient += levelTimeGradient * levelTimeGradientWeight;
             }
 

diff --git a/.../java/ai/timefold/solver/core/impl/solver/termination/ChildThreadPlumbingTermination.java b/.../java/ai/timefold/solver/core/impl/solver/termination/ChildThreadPlumbingTermination.java
@@ -4,9 +4,9 @@
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
 import ai.timefold.solver.core.impl.solver.thread.ChildThreadType;
 
-public class ChildThreadPlumbingTermination<Solution_> extends AbstractTermination<Solution_> {
+public final class ChildThreadPlumbingTermination<Solution_> extends AbstractTermination<Solution_> {
 
-    protected boolean terminateChildren = false;
+    private boolean terminateChildren = false;
 
     // ************************************************************************
     // Plumbing worker methods
@@ -18,7 +18,7 @@ public class ChildThreadPlumbingTermination<Solution_> extends AbstractTerminati
      * @return true if termination hasn't been requested previously
      */
     public synchronized boolean terminateChildren() {
-        boolean terminationEarlySuccessful = !terminateChildren;
+        var terminationEarlySuccessful = !terminateChildren;
         terminateChildren = true;
         return terminationEarlySuccessful;
     }

diff --git a/...src/main/java/ai/timefold/solver/core/impl/solver/termination/OrCompositeTermination.java b/...src/main/java/ai/timefold/solver/core/impl/solver/termination/OrCompositeTermination.java
@@ -6,7 +6,7 @@
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
 import ai.timefold.solver.core.impl.solver.thread.ChildThreadType;
 
-public class OrCompositeTermination<Solution_> extends AbstractCompositeTermination<Solution_> {
+public final class OrCompositeTermination<Solution_> extends AbstractCompositeTermination<Solution_> {
 
     public OrCompositeTermination(List<Termination<Solution_>> terminationList) {
         super(terminationList);

diff --git a/.../java/ai/timefold/solver/core/impl/solver/termination/PhaseToSolverTerminationBridge.java b/.../java/ai/timefold/solver/core/impl/solver/termination/PhaseToSolverTerminationBridge.java
@@ -4,7 +4,7 @@
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
 import ai.timefold.solver.core.impl.solver.thread.ChildThreadType;
 
-public class PhaseToSolverTerminationBridge<Solution_> extends AbstractTermination<Solution_> {
+public final class PhaseToSolverTerminationBridge<Solution_> extends AbstractTermination<Solution_> {
 
     private final Termination<Solution_> solverTermination;
 

diff --git a/...ava/ai/timefold/solver/core/impl/solver/termination/ScoreCalculationCountTermination.java b/...ava/ai/timefold/solver/core/impl/solver/termination/ScoreCalculationCountTermination.java
@@ -5,15 +5,16 @@
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
 import ai.timefold.solver.core.impl.solver.thread.ChildThreadType;
 
-public class ScoreCalculationCountTermination<Solution_> extends AbstractTermination<Solution_> {
+public final class ScoreCalculationCountTermination<Solution_> extends AbstractTermination<Solution_> {
 
     private final long scoreCalculationCountLimit;
 
     public ScoreCalculationCountTermination(long scoreCalculationCountLimit) {
         this.scoreCalculationCountLimit = scoreCalculationCountLimit;
         if (scoreCalculationCountLimit < 0L) {
-            throw new IllegalArgumentException("The scoreCalculationCountLimit (" + scoreCalculationCountLimit
-                    + ") cannot be negative.");
+            throw new IllegalArgumentException(
+                    "The scoreCalculationCountLimit (%d) cannot be negative."
+                            .formatted(scoreCalculationCountLimit));
         }
     }
 
@@ -31,8 +32,8 @@ public boolean isPhaseTerminated(AbstractPhaseScope<Solution_> phaseScope) {
         return isTerminated(phaseScope.getScoreDirector());
     }
 
-    protected boolean isTerminated(InnerScoreDirector<Solution_, ?> scoreDirector) {
-        long scoreCalculationCount = scoreDirector.getCalculationCount();
+    private boolean isTerminated(InnerScoreDirector<Solution_, ?> scoreDirector) {
+        var scoreCalculationCount = scoreDirector.getCalculationCount();
         return scoreCalculationCount >= scoreCalculationCountLimit;
     }
 
@@ -50,9 +51,9 @@ public double calculatePhaseTimeGradient(AbstractPhaseScope<Solution_> phaseScop
         return calculateTimeGradient(phaseScope.getScoreDirector());
     }
 
-    protected double calculateTimeGradient(InnerScoreDirector<Solution_, ?> scoreDirector) {
-        long scoreCalculationCount = scoreDirector.getCalculationCount();
-        double timeGradient = scoreCalculationCount / ((double) scoreCalculationCountLimit);
+    private double calculateTimeGradient(InnerScoreDirector<Solution_, ?> scoreDirector) {
+        var scoreCalculationCount = scoreDirector.getCalculationCount();
+        var timeGradient = scoreCalculationCount / ((double) scoreCalculationCountLimit);
         return Math.min(timeGradient, 1.0);
     }
 
@@ -67,7 +68,8 @@ public ScoreCalculationCountTermination<Solution_> createChildThreadTermination(
             // The ScoreDirector.calculationCount of partitions is maxed, not summed.
             return new ScoreCalculationCountTermination<>(scoreCalculationCountLimit);
         } else {
-            throw new IllegalStateException("The childThreadType (" + childThreadType + ") is not implemented.");
+            throw new IllegalStateException("The childThreadType (%s) is not implemented."
+                    .formatted(childThreadType));
         }
     }