fix: phases should always check if the solver terminated early (#1456)

Fixes a regression where terminateEarly was not checked until the end of
a phase.
Previously, the top level termination was always
BasicPlumbingTermination, which checks for/sets the terminateEarly flag.
However, with the refactor to terminations, phases use different
termination instances (that are not instances of
BasicPlumbingTermination).
None of these terminations check terminateEarly before, since that was
the job of the BasicPlumbingTermination.

We fix that by making plumbing terminations always check for solver
termination.

Note: this PR is #1455 with additional tests.

---------

Co-authored-by: Lukáš Petrovický <[email protected]>

Author: Christopher-Chianelli

core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhaseFactory.java

@@ -40,9 +40,10 @@ protected PhaseTermination<Solution_> buildPhaseTermination(HeuristicConfigPolic
         var phaseTermination = PhaseTermination.bridge(solverTermination);
         var resultingTermination = TerminationFactory.<Solution_> create(terminationConfig_)
                 .buildTermination(configPolicy, phaseTermination);
-        var inapplicableTerminationList = resultingTermination instanceof UniversalTermination<Solution_> universalTermination
-                ? universalTermination.getPhaseTerminationsInapplicableTo(getPhaseScopeClass())
-                : Collections.emptyList();
+        var inapplicableTerminationList = !(this instanceof NoChangePhaseFactory<?>) &&
+                resultingTermination instanceof UniversalTermination<Solution_> universalTermination
+                        ? universalTermination.getPhaseTerminationsInapplicableTo(getPhaseScopeClass())
+                        : Collections.emptyList();
         var phaseName = this.getClass().getSimpleName()
                 .replace("PhaseFactory", "")
                 .replace("Default", "");

core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolverFactory.java

@@ -136,7 +136,7 @@ public <Score_ extends Score<Score_>> InnerScoreDirectorFactory<Solution_, Score
         var phaseList = buildPhaseList(configPolicy, bestSolutionRecaller, termination);
 
         return new DefaultSolver<>(environmentMode, randomFactory, bestSolutionRecaller, basicPlumbingTermination,
-                UniversalTermination.bridge(termination), phaseList, solverScope,
+                (UniversalTermination<Solution_>) termination, phaseList, solverScope,
                 moveThreadCount == null ? SolverConfig.MOVE_THREAD_COUNT_NONE : Integer.toString(moveThreadCount));
     }
 

core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AbstractSolverTermination.java

@@ -7,7 +7,7 @@
 @NullMarked
 abstract sealed class AbstractTermination<Solution_>
         implements Termination<Solution_>
-        permits AbstractPhaseTermination, AbstractUniversalTermination, AbstractSolverTermination {
+        permits AbstractPhaseTermination, AbstractUniversalTermination {
 
     protected final transient Logger logger = LoggerFactory.getLogger(getClass());
 

core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AbstractTermination.java

@@ -10,8 +10,8 @@
 abstract sealed class AbstractUniversalTermination<Solution_>
         extends AbstractTermination<Solution_>
         implements UniversalTermination<Solution_>
-        permits AbstractCompositeTermination, BestScoreFeasibleTermination, BestScoreTermination, MoveCountTermination,
-        ScoreCalculationCountTermination, SolverToUniversalBridgeTermination, TimeMillisSpentTermination,
+        permits AbstractCompositeTermination, BasicPlumbingTermination, BestScoreFeasibleTermination, BestScoreTermination,
+        ChildThreadPlumbingTermination, MoveCountTermination, ScoreCalculationCountTermination, TimeMillisSpentTermination,
         UnimprovedTimeMillisSpentScoreDifferenceThresholdTermination, UnimprovedTimeMillisSpentTermination {
 
     @Override

core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AbstractUniversalTermination.java

@@ -5,6 +5,7 @@
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
 
+import ai.timefold.solver.core.impl.phase.scope.AbstractPhaseScope;
 import ai.timefold.solver.core.impl.solver.change.ProblemChangeAdapter;
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
 import ai.timefold.solver.core.impl.solver.thread.ChildThreadType;
@@ -17,7 +18,7 @@
  */
 @NullMarked
 public final class BasicPlumbingTermination<Solution_>
-        extends AbstractSolverTermination<Solution_>
+        extends AbstractUniversalTermination<Solution_>
         implements ChildThreadSupportingTermination<Solution_, SolverScope<Solution_>> {
 
     private final boolean daemon;
@@ -126,6 +127,16 @@ public double calculateSolverTimeGradient(SolverScope<Solution_> solverScope) {
         return -1.0; // Not supported
     }
 
+    @Override
+    public boolean isPhaseTerminated(AbstractPhaseScope<Solution_> phaseScope) {
+        return isSolverTerminated(phaseScope.getSolverScope());
+    }
+
+    @Override
+    public double calculatePhaseTimeGradient(AbstractPhaseScope<Solution_> phaseScope) {
+        return calculateSolverTimeGradient(phaseScope.getSolverScope());
+    }
+
     @Override
     public Termination<Solution_> createChildThreadTermination(SolverScope<Solution_> solverScope,
             ChildThreadType childThreadType) {

core/src/main/java/ai/timefold/solver/core/impl/solver/termination/BasicPlumbingTermination.java

@@ -1,13 +1,14 @@
 package ai.timefold.solver.core.impl.solver.termination;
 
+import ai.timefold.solver.core.impl.phase.scope.AbstractPhaseScope;
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
 import ai.timefold.solver.core.impl.solver.thread.ChildThreadType;
 
 import org.jspecify.annotations.NullMarked;
 
 @NullMarked
 public final class ChildThreadPlumbingTermination<Solution_>
-        extends AbstractSolverTermination<Solution_>
+        extends AbstractUniversalTermination<Solution_>
         implements ChildThreadSupportingTermination<Solution_, SolverScope<Solution_>> {
 
     private boolean terminateChildren = false;
@@ -38,6 +39,16 @@ public double calculateSolverTimeGradient(SolverScope<Solution_> solverScope) {
         return -1.0; // Not supported
     }
 
+    @Override
+    public boolean isPhaseTerminated(AbstractPhaseScope<Solution_> phaseScope) {
+        return isSolverTerminated(phaseScope.getSolverScope());
+    }
+
+    @Override
+    public double calculatePhaseTimeGradient(AbstractPhaseScope<Solution_> phaseScope) {
+        return calculateSolverTimeGradient(phaseScope.getSolverScope());
+    }
+
     @Override
     public Termination<Solution_> createChildThreadTermination(SolverScope<Solution_> solverScope,
             ChildThreadType childThreadType) {

core/src/main/java/ai/timefold/solver/core/impl/solver/termination/ChildThreadPlumbingTermination.java

@@ -13,7 +13,7 @@
 @NullMarked
 public sealed interface SolverTermination<Solution_>
         extends Termination<Solution_>, SolverLifecycleListener<Solution_>
-        permits AbstractSolverTermination, MockableSolverTermination, UniversalTermination {
+        permits MockableSolverTermination, UniversalTermination {
 
     /**
      * Called by the {@link Solver} after every phase to determine if the search should stop.

core/src/main/java/ai/timefold/solver/core/impl/solver/termination/SolverTermination.java

@@ -58,12 +58,4 @@ default List<PhaseTermination<Solution_>> getPhaseTerminationList() {
         return new AndCompositeTermination<>(terminations);
     }
 
-    static <Solution_> UniversalTermination<Solution_> bridge(SolverTermination<Solution_> termination) {
-        if (termination instanceof UniversalTermination<Solution_> universalTermination) {
-            return universalTermination;
-        } else {
-            return new SolverToUniversalBridgeTermination<>(termination);
-        }
-    }
-
 }