From d0ddf2d9bf3ddfe6fd098fa879d52383813e593b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Fri, 21 Feb 2025 08:34:29 +0100 Subject: [PATCH 01/14] chore: fail fast when score corruption detected --- benchmark/src/main/resources/benchmark.xsd | 3 + .../core/config/solver/EnvironmentMode.java | 26 +++++- .../DefaultConstructionHeuristicPhase.java | 7 ++ ...aultConstructionHeuristicPhaseFactory.java | 14 +-- .../DefaultExhaustiveSearchPhase.java | 13 +-- .../DefaultExhaustiveSearchPhaseFactory.java | 18 +--- .../localsearch/DefaultLocalSearchPhase.java | 7 ++ .../DefaultLocalSearchPhaseFactory.java | 22 ++--- .../decider/LocalSearchDecider.java | 10 +-- .../solver/core/impl/phase/AbstractPhase.java | 40 ++++++--- .../impl/phase/custom/DefaultCustomPhase.java | 7 ++ .../custom/DefaultCustomPhaseFactory.java | 11 +-- .../score/director/AbstractScoreDirector.java | 90 ++++++++++--------- .../exception/CloningCorruptionException.java | 7 ++ .../exception/ScoreCorruptionException.java | 7 ++ .../UndoScoreCorruptionException.java | 12 ++- .../VariableCorruptionException.java | 7 ++ .../impl/solver/exception/package-info.java | 6 ++ core/src/main/resources/solver.xsd | 2 + .../core/api/solver/SolverManagerTest.java | 2 + .../config/solver/EnvironmentModeTest.java | 42 +++------ .../core/impl/solver/DefaultSolverTest.java | 30 +++++++ 22 files changed, 233 insertions(+), 150 deletions(-) create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/solver/exception/CloningCorruptionException.java create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/solver/exception/ScoreCorruptionException.java create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/solver/exception/VariableCorruptionException.java create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/solver/exception/package-info.java diff --git a/benchmark/src/main/resources/benchmark.xsd b/benchmark/src/main/resources/benchmark.xsd index 24305f6e35a..cc4a93a888c 100644 --- a/benchmark/src/main/resources/benchmark.xsd +++ b/benchmark/src/main/resources/benchmark.xsd @@ -2633,6 +2633,9 @@ + + + diff --git a/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java b/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java index 60d4c1992dd..3d2c63ad159 100644 --- a/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java +++ b/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java @@ -89,21 +89,39 @@ public enum EnvironmentMode { */ REPRODUCIBLE(false), /** - * The non reproducible mode is equally fast or slightly faster than {@link #REPRODUCIBLE}. + * As defined by {@link #REPRODUCIBLE}, but it also disables certain bug detection mechanisms. + * This mode will run marginally faster than {@link #REPRODUCIBLE}, + * but will allow some bugs in user code (such as score corruptions) to go unnoticed. + * Use this mode during development when you are confident that your code is bug-free, + * or when you want to ignore a known bug temporarily. + */ + REPRODUCIBLE_UNGUARDED(false, false), + /** + * The non-reproducible mode is equally fast or slightly faster than {@link #REPRODUCIBLE_UNGUARDED}. *

* The random seed is different on every run, which makes it more robust against an unlucky random seed. * An unlucky random seed gives a bad result on a certain data set with a certain solver configuration. - * Note that in most use cases the impact of the random seed is relatively low on the result. + * Note that in most use cases, the impact of the random seed is relatively low on the result. * An occasional bad result is far more likely to be caused by another issue (such as a score trap). *

- * In multithreaded scenarios, this mode allows the use of work stealing and other non deterministic speed tricks. + * In multithreaded scenarios, this mode allows the use of work stealing and other non-deterministic speed tricks. */ - NON_REPRODUCIBLE(false); + NON_REPRODUCIBLE(false, false); private final boolean asserted; + private final boolean guarded; EnvironmentMode(boolean asserted) { + this(asserted, true); + } + + EnvironmentMode(boolean asserted, boolean guarded) { this.asserted = asserted; + this.guarded = guarded; + } + + public boolean isGuarded() { + return guarded; } public boolean isAsserted() { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java b/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java index 3e926e0325f..65490688389 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java @@ -1,6 +1,7 @@ package ai.timefold.solver.core.impl.constructionheuristic; import ai.timefold.solver.core.api.domain.solution.PlanningSolution; +import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.impl.constructionheuristic.decider.ConstructionHeuristicDecider; import ai.timefold.solver.core.impl.constructionheuristic.placer.EntityPlacer; import ai.timefold.solver.core.impl.constructionheuristic.scope.ConstructionHeuristicPhaseScope; @@ -220,6 +221,12 @@ public DefaultConstructionHeuristicPhaseBuilder(int phaseIndex, boolean lastInit this.decider = decider; } + @Override + public DefaultConstructionHeuristicPhaseBuilder enableAssertions(EnvironmentMode environmentMode) { + super.enableAssertions(environmentMode); + return this; + } + public EntityPlacer getEntityPlacer() { return entityPlacer; } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseFactory.java index 88af23e5645..c4160597250 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseFactory.java @@ -69,18 +69,10 @@ protected DefaultConstructionHeuristicPhaseBuilder createBuilder( HeuristicConfigPolicy phaseConfigPolicy, Termination solverTermination, int phaseIndex, boolean lastInitializingPhase, EntityPlacer entityPlacer) { var phaseTermination = buildPhaseTermination(phaseConfigPolicy, solverTermination); - var builder = new DefaultConstructionHeuristicPhaseBuilder<>(phaseIndex, lastInitializingPhase, + return new DefaultConstructionHeuristicPhaseBuilder<>(phaseIndex, lastInitializingPhase, phaseConfigPolicy.getLogIndentation(), phaseTermination, entityPlacer, - buildDecider(phaseConfigPolicy, phaseTermination)); - var environmentMode = phaseConfigPolicy.getEnvironmentMode(); - if (environmentMode.isNonIntrusiveFullAsserted()) { - builder.setAssertStepScoreFromScratch(true); - } - if (environmentMode.isIntrusiveFastAsserted()) { - builder.setAssertExpectedStepScore(true); - builder.setAssertShadowVariablesAreNotStaleAfterStep(true); - } - return builder; + buildDecider(phaseConfigPolicy, phaseTermination)) + .enableAssertions(phaseConfigPolicy.getEnvironmentMode()); } @Override diff --git a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java index 08551fcf33c..d8a74328318 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java @@ -9,6 +9,7 @@ import ai.timefold.solver.core.api.domain.solution.PlanningSolution; import ai.timefold.solver.core.api.score.Score; +import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.impl.exhaustivesearch.decider.ExhaustiveSearchDecider; import ai.timefold.solver.core.impl.exhaustivesearch.node.ExhaustiveSearchLayer; import ai.timefold.solver.core.impl.exhaustivesearch.node.ExhaustiveSearchNode; @@ -244,12 +245,12 @@ public Builder(int phaseIndex, String logIndentation, Termination pha this.decider = decider; } - public void setAssertWorkingSolutionScoreFromScratch(boolean assertWorkingSolutionScoreFromScratch) { - this.assertWorkingSolutionScoreFromScratch = assertWorkingSolutionScoreFromScratch; - } - - public void setAssertExpectedWorkingSolutionScore(boolean assertExpectedWorkingSolutionScore) { - this.assertExpectedWorkingSolutionScore = assertExpectedWorkingSolutionScore; + @Override + public Builder enableAssertions(EnvironmentMode environmentMode) { + super.enableAssertions(environmentMode); + assertWorkingSolutionScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); + assertExpectedWorkingSolutionScore = environmentMode.isIntrusiveFastAsserted(); + return this; } @Override diff --git a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java index 3ad15951154..ba929407c7a 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java @@ -83,22 +83,12 @@ public ExhaustiveSearchPhase buildPhase(int phaseIndex, boolean lastI EntitySelectorFactory. create(entitySelectorConfig_) .buildEntitySelector(phaseConfigPolicy, SelectionCacheType.PHASE, SelectionOrder.ORIGINAL); - DefaultExhaustiveSearchPhase.Builder builder = new DefaultExhaustiveSearchPhase.Builder<>(phaseIndex, + return new DefaultExhaustiveSearchPhase.Builder<>(phaseIndex, solverConfigPolicy.getLogIndentation(), phaseTermination, nodeExplorationType_.buildNodeComparator(scoreBounderEnabled), entitySelector, buildDecider(phaseConfigPolicy, - entitySelector, bestSolutionRecaller, phaseTermination, scoreBounderEnabled)); - - EnvironmentMode environmentMode = phaseConfigPolicy.getEnvironmentMode(); - if (environmentMode.isNonIntrusiveFullAsserted()) { - builder.setAssertWorkingSolutionScoreFromScratch(true); - builder.setAssertStepScoreFromScratch(true); // Does nothing because ES doesn't use predictStepScore() - } - if (environmentMode.isIntrusiveFastAsserted()) { - builder.setAssertExpectedWorkingSolutionScore(true); - builder.setAssertExpectedStepScore(true); // Does nothing because ES doesn't use predictStepScore() - builder.setAssertShadowVariablesAreNotStaleAfterStep(true); // Does nothing because ES doesn't use predictStepScore() - } - return builder.build(); + entitySelector, bestSolutionRecaller, phaseTermination, scoreBounderEnabled)) + .enableAssertions(phaseConfigPolicy.getEnvironmentMode()) + .build(); } private EntitySelectorConfig buildEntitySelectorConfig(HeuristicConfigPolicy configPolicy) { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhase.java b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhase.java index c2a53cdbcf5..fcc7cb89390 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhase.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhase.java @@ -8,6 +8,7 @@ import ai.timefold.solver.core.api.domain.solution.PlanningSolution; import ai.timefold.solver.core.api.score.constraint.ConstraintMatchTotal; +import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.config.solver.monitoring.SolverMetric; import ai.timefold.solver.core.impl.localsearch.decider.LocalSearchDecider; import ai.timefold.solver.core.impl.localsearch.event.LocalSearchPhaseLifecycleListener; @@ -235,6 +236,12 @@ public Builder(int phaseIndex, String logIndentation, Termination pha this.decider = decider; } + @Override + public Builder enableAssertions(EnvironmentMode environmentMode) { + super.enableAssertions(environmentMode); + return this; + } + @Override public DefaultLocalSearchPhase build() { return new DefaultLocalSearchPhase<>(this); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseFactory.java index 22bd5d9986d..e616f8cbb52 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseFactory.java @@ -48,17 +48,10 @@ public LocalSearchPhase buildPhase(int phaseIndex, boolean lastInitia Termination solverTermination) { var phaseConfigPolicy = solverConfigPolicy.createPhaseConfigPolicy(); var phaseTermination = buildPhaseTermination(phaseConfigPolicy, solverTermination); - var builder = new DefaultLocalSearchPhase.Builder<>(phaseIndex, solverConfigPolicy.getLogIndentation(), - phaseTermination, buildDecider(phaseConfigPolicy, phaseTermination)); - var environmentMode = phaseConfigPolicy.getEnvironmentMode(); - if (environmentMode.isNonIntrusiveFullAsserted()) { - builder.setAssertStepScoreFromScratch(true); - } - if (environmentMode.isIntrusiveFastAsserted()) { - builder.setAssertExpectedStepScore(true); - builder.setAssertShadowVariablesAreNotStaleAfterStep(true); - } - return builder.build(); + return new DefaultLocalSearchPhase.Builder<>(phaseIndex, solverConfigPolicy.getLogIndentation(), + phaseTermination, buildDecider(phaseConfigPolicy, phaseTermination)) + .enableAssertions(phaseConfigPolicy.getEnvironmentMode()) + .build(); } private LocalSearchDecider buildDecider(HeuristicConfigPolicy configPolicy, @@ -83,12 +76,7 @@ private LocalSearchDecider buildDecider(HeuristicConfigPolicy getForager() { return forager; } - public void setAssertMoveScoreFromScratch(boolean assertMoveScoreFromScratch) { - this.assertMoveScoreFromScratch = assertMoveScoreFromScratch; - } - - public void setAssertExpectedUndoMoveScore(boolean assertExpectedUndoMoveScore) { - this.assertExpectedUndoMoveScore = assertExpectedUndoMoveScore; + public void enableAssertions(EnvironmentMode environmentMode) { + assertMoveScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); + assertExpectedUndoMoveScore = environmentMode.isIntrusiveFastAsserted(); } // ************************************************************************ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java b/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java index d2683ad43e3..2ffe47a25ba 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java @@ -2,6 +2,7 @@ import ai.timefold.solver.core.api.domain.solution.PlanningSolution; import ai.timefold.solver.core.api.score.Score; +import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.config.solver.monitoring.SolverMetric; import ai.timefold.solver.core.impl.localsearch.DefaultLocalSearchPhase; import ai.timefold.solver.core.impl.phase.event.PhaseLifecycleListener; @@ -9,6 +10,8 @@ import ai.timefold.solver.core.impl.phase.scope.AbstractPhaseScope; import ai.timefold.solver.core.impl.phase.scope.AbstractStepScope; import ai.timefold.solver.core.impl.solver.AbstractSolver; +import ai.timefold.solver.core.impl.solver.exception.ScoreCorruptionException; +import ai.timefold.solver.core.impl.solver.exception.VariableCorruptionException; import ai.timefold.solver.core.impl.solver.scope.SolverScope; import ai.timefold.solver.core.impl.solver.termination.Termination; @@ -29,6 +32,7 @@ public abstract class AbstractPhase implements Phase { // Called "phaseTermination" to clearly distinguish from "solverTermination" inside AbstractSolver. protected final Termination phaseTermination; + protected final boolean assertPhaseScoreFromScratch; protected final boolean assertStepScoreFromScratch; protected final boolean assertExpectedStepScore; protected final boolean assertShadowVariablesAreNotStaleAfterStep; @@ -42,6 +46,7 @@ protected AbstractPhase(AbstractPhaseBuilder builder) { phaseIndex = builder.phaseIndex; logIndentation = builder.logIndentation; phaseTermination = builder.phaseTermination; + assertPhaseScoreFromScratch = builder.assertPhaseScoreFromScratch; assertStepScoreFromScratch = builder.assertStepScoreFromScratch; assertExpectedStepScore = builder.assertExpectedStepScore; assertShadowVariablesAreNotStaleAfterStep = builder.assertShadowVariablesAreNotStaleAfterStep; @@ -124,6 +129,24 @@ public void phaseEnded(AbstractPhaseScope phaseScope) { } phaseTermination.phaseEnded(phaseScope); phaseLifecycleSupport.firePhaseEnded(phaseScope); + if (assertPhaseScoreFromScratch) { + var score = phaseScope.getSolverScope().calculateScore(); + try { + phaseScope.assertWorkingScoreFromScratch(score, getPhaseTypeString() + " phase ended"); + } catch (ScoreCorruptionException | VariableCorruptionException e) { + throw new IllegalStateException(""" + Solver corruption was detected. Solutions provided by this solver can not be trusted. + Corruptions typically arise from a bug in either your constraints or your variable listeners, + but they may also be caused by a rare solver bug. + Run your solver with %s %s to find out more information about the error \ + and if you are convinced that the problem is not in your code, please report a bug to Timefold. + At your own risk, you may run your solver with %s or %s instead to ignore this error.""" + .formatted(EnvironmentMode.class.getSimpleName(), + EnvironmentMode.FULL_ASSERT, EnvironmentMode.REPRODUCIBLE_UNGUARDED, + EnvironmentMode.NON_REPRODUCIBLE), + e); + } + } } @Override @@ -234,6 +257,7 @@ public abstract static class AbstractPhaseBuilder { private final String logIndentation; private final Termination phaseTermination; + private boolean assertPhaseScoreFromScratch = false; private boolean assertStepScoreFromScratch = false; private boolean assertExpectedStepScore = false; private boolean assertShadowVariablesAreNotStaleAfterStep = false; @@ -244,16 +268,12 @@ protected AbstractPhaseBuilder(int phaseIndex, String logIndentation, Terminatio this.phaseTermination = phaseTermination; } - public void setAssertStepScoreFromScratch(boolean assertStepScoreFromScratch) { - this.assertStepScoreFromScratch = assertStepScoreFromScratch; - } - - public void setAssertExpectedStepScore(boolean assertExpectedStepScore) { - this.assertExpectedStepScore = assertExpectedStepScore; - } - - public void setAssertShadowVariablesAreNotStaleAfterStep(boolean assertShadowVariablesAreNotStaleAfterStep) { - this.assertShadowVariablesAreNotStaleAfterStep = assertShadowVariablesAreNotStaleAfterStep; + public AbstractPhaseBuilder enableAssertions(EnvironmentMode environmentMode) { + assertPhaseScoreFromScratch = environmentMode.isGuarded() && !environmentMode.isAsserted(); + assertStepScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); + assertExpectedStepScore = environmentMode.isIntrusiveFastAsserted(); + assertShadowVariablesAreNotStaleAfterStep = environmentMode.isIntrusiveFastAsserted(); + return this; } protected abstract AbstractPhase build(); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhase.java b/core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhase.java index f5dc7ad5127..c5ef2f1a5fc 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhase.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhase.java @@ -4,6 +4,7 @@ import java.util.List; import ai.timefold.solver.core.api.domain.solution.PlanningSolution; +import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.impl.phase.AbstractPossiblyInitializingPhase; import ai.timefold.solver.core.impl.phase.custom.scope.CustomPhaseScope; import ai.timefold.solver.core.impl.phase.custom.scope.CustomStepScope; @@ -122,6 +123,12 @@ public DefaultCustomPhaseBuilder(int phaseIndex, boolean lastInitializingPhase, this.customPhaseCommandList = List.copyOf(customPhaseCommandList); } + @Override + public DefaultCustomPhaseBuilder enableAssertions(EnvironmentMode environmentMode) { + super.enableAssertions(environmentMode); + return this; + } + @Override public DefaultCustomPhase build() { return new DefaultCustomPhase<>(this); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhaseFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhaseFactory.java index 264e8b9ee11..17cd1f9eb6b 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhaseFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhaseFactory.java @@ -44,14 +44,11 @@ The customPhaseCommandClass (%s) cannot be null in the customPhase (%s). if (customPhaseCommandList != null) { customPhaseCommandList_.addAll((Collection) customPhaseCommandList); } - var builder = new DefaultCustomPhase.DefaultCustomPhaseBuilder<>(phaseIndex, lastInitializingPhase, + return new DefaultCustomPhase.DefaultCustomPhaseBuilder<>(phaseIndex, lastInitializingPhase, solverConfigPolicy.getLogIndentation(), buildPhaseTermination(phaseConfigPolicy, solverTermination), - customPhaseCommandList_); - var environmentMode = phaseConfigPolicy.getEnvironmentMode(); - if (environmentMode.isNonIntrusiveFullAsserted()) { - builder.setAssertStepScoreFromScratch(true); - } - return builder.build(); + customPhaseCommandList_) + .enableAssertions(phaseConfigPolicy.getEnvironmentMode()) + .build(); } private CustomPhaseCommand diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java index 6d691d449c2..69d95ff0da8 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java @@ -36,7 +36,10 @@ import ai.timefold.solver.core.impl.phase.scope.SolverLifecyclePoint; import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; import ai.timefold.solver.core.impl.score.definition.ScoreDefinition; +import ai.timefold.solver.core.impl.solver.exception.CloningCorruptionException; +import ai.timefold.solver.core.impl.solver.exception.ScoreCorruptionException; import ai.timefold.solver.core.impl.solver.exception.UndoScoreCorruptionException; +import ai.timefold.solver.core.impl.solver.exception.VariableCorruptionException; import ai.timefold.solver.core.impl.solver.thread.ChildThreadType; import ai.timefold.solver.core.preview.api.move.Move; @@ -295,22 +298,21 @@ public Solution_ cloneSolution(Solution_ originalSolution) { Score_ cloneScore = solutionDescriptor.getScore(cloneSolution); if (scoreDirectorFactory.isAssertClonedSolution()) { if (!Objects.equals(originalScore, cloneScore)) { - throw new IllegalStateException("Cloning corruption: " - + "the original's score (" + originalScore - + ") is different from the clone's score (" + cloneScore + ").\n" - + "Check the " + SolutionCloner.class.getSimpleName() + "."); + throw new CloningCorruptionException(""" + Cloning corruption: the original's score (%s) is different from the clone's score (%s). + Check the %s.""" + .formatted(originalScore, cloneScore, SolutionCloner.class.getSimpleName())); } Map originalEntityMap = new IdentityHashMap<>(); solutionDescriptor.visitAllEntities(originalSolution, originalEntity -> originalEntityMap.put(originalEntity, null)); solutionDescriptor.visitAllEntities(cloneSolution, cloneEntity -> { if (originalEntityMap.containsKey(cloneEntity)) { - throw new IllegalStateException("Cloning corruption: " - + "the same entity (" + cloneEntity - + ") is present in both the original and the clone.\n" - + "So when a planning variable in the original solution changes, " - + "the cloned solution will change too.\n" - + "Check the " + SolutionCloner.class.getSimpleName() + "."); + throw new CloningCorruptionException(""" + Cloning corruption: the same entity (%s) is present in both the original and the clone. + So when a planning variable in the original solution changes, the cloned solution will change too. + Check the %s.""" + .formatted(cloneEntity, SolutionCloner.class.getSimpleName())); } }); } @@ -557,11 +559,11 @@ public void afterProblemFactRemoved(Object problemFact) { public void assertExpectedWorkingScore(Score_ expectedWorkingScore, Object completedAction) { Score_ workingScore = calculateScore(); if (!expectedWorkingScore.equals(workingScore)) { - throw new IllegalStateException( - "Score corruption (" + expectedWorkingScore.subtract(workingScore).toShortString() - + "): the expectedWorkingScore (" + expectedWorkingScore - + ") is not the workingScore (" + workingScore - + ") after completedAction (" + completedAction + ")."); + throw new ScoreCorruptionException(""" + Score corruption (%s): the expectedWorkingScore (%s) is not the workingScore (%s) \ + after completedAction (%s).""" + .formatted(expectedWorkingScore.subtract(workingScore).toShortString(), + expectedWorkingScore, workingScore, completedAction)); } } @@ -569,26 +571,25 @@ public void assertExpectedWorkingScore(Score_ expectedWorkingScore, Object compl public void assertShadowVariablesAreNotStale(Score_ expectedWorkingScore, Object completedAction) { String violationMessage = variableListenerSupport.createShadowVariablesViolationMessage(); if (violationMessage != null) { - throw new IllegalStateException( - VariableListener.class.getSimpleName() + " corruption after completedAction (" - + completedAction + "):\n" - + violationMessage); + throw new VariableCorruptionException(""" + %s corruption after completedAction (%s): + %s""" + .formatted(VariableListener.class.getSimpleName(), completedAction, violationMessage)); } Score_ workingScore = calculateScore(); if (!expectedWorkingScore.equals(workingScore)) { assertWorkingScoreFromScratch(workingScore, "assertShadowVariablesAreNotStale(" + expectedWorkingScore + ", " + completedAction + ")"); - throw new IllegalStateException("Impossible " + VariableListener.class.getSimpleName() + " corruption (" - + expectedWorkingScore.subtract(workingScore).toShortString() + "):" - + " the expectedWorkingScore (" + expectedWorkingScore - + ") is not the workingScore (" + workingScore - + ") after all " + VariableListener.class.getSimpleName() - + "s were triggered without changes to the genuine variables" - + " after completedAction (" + completedAction + ").\n" - + "But all the shadow variable values are still the same, so this is impossible.\n" - + "Maybe run with " + EnvironmentMode.TRACKED_FULL_ASSERT - + " if you aren't already, to fail earlier."); + throw new VariableCorruptionException(""" + Impossible %s corruption (%s): the expectedWorkingScore (%s) is not the workingScore (%s) \ + after all %ss were triggered without changes to the genuine variables after completedAction (%s). + All the shadow variable values are still the same, so this is impossible. + Maybe run with %s if you haven't already, to fail earlier.""" + .formatted(VariableListener.class.getSimpleName(), + expectedWorkingScore.subtract(workingScore).toShortString(), + expectedWorkingScore, workingScore, VariableListener.class.getSimpleName(), completedAction, + EnvironmentMode.TRACKED_FULL_ASSERT)); } } @@ -600,13 +601,16 @@ protected String buildShadowVariableAnalysis(boolean predicted) { String violationMessage = variableListenerSupport.createShadowVariablesViolationMessage(); String workingLabel = predicted ? "working" : "corrupted"; if (violationMessage == null) { - return "Shadow variable corruption in the " + workingLabel + " scoreDirector:\n" - + " None"; + return """ + Shadow variable corruption in the %s scoreDirector: + None""" + .formatted(workingLabel); } - return "Shadow variable corruption in the " + workingLabel + " scoreDirector:\n" - + violationMessage - + " Maybe there is a bug in the " + VariableListener.class.getSimpleName() - + " of those shadow variable(s)."; + return """ + Shadow variable corruption in the %s scoreDirector: + %s + Maybe there is a bug in the %s of those shadow variable(s).""" + .formatted(workingLabel, violationMessage, VariableListener.class.getSimpleName()); } @Override @@ -632,13 +636,13 @@ private void assertScoreFromScratch(Score_ score, Object completedAction, boolea if (!score.equals(uncorruptedScore)) { String scoreCorruptionAnalysis = buildScoreCorruptionAnalysis(uncorruptedScoreDirector, predicted); String shadowVariableAnalysis = buildShadowVariableAnalysis(predicted); - throw new IllegalStateException( - "Score corruption (" + score.subtract(uncorruptedScore).toShortString() - + "): the " + (predicted ? "predictedScore" : "workingScore") + " (" + score - + ") is not the uncorruptedScore (" + uncorruptedScore - + ") after completedAction (" + completedAction + "):\n" - + scoreCorruptionAnalysis + "\n" - + shadowVariableAnalysis); + throw new ScoreCorruptionException(""" + Score corruption (%s): the %s (%s) is not the uncorruptedScore (%s) after completedAction (%s): + %s + %s""" + .formatted(score.subtract(uncorruptedScore).toShortString(), + predicted ? "predictedScore" : "workingScore", score, uncorruptedScore, completedAction, + scoreCorruptionAnalysis, shadowVariableAnalysis)); } } } @@ -707,7 +711,7 @@ that could cause the scoreDifference (%s).""" solutionTracker.getAfterMoveSolution(), solutionTracker.getAfterUndoSolution()); } else { - throw new IllegalStateException(corruptionMessage); + throw new ScoreCorruptionException(corruptionMessage); } } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/CloningCorruptionException.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/CloningCorruptionException.java new file mode 100644 index 00000000000..78ff77186c0 --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/CloningCorruptionException.java @@ -0,0 +1,7 @@ +package ai.timefold.solver.core.impl.solver.exception; + +public final class CloningCorruptionException extends IllegalStateException { + public CloningCorruptionException(String message) { + super(message); + } +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/ScoreCorruptionException.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/ScoreCorruptionException.java new file mode 100644 index 00000000000..87f27455212 --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/ScoreCorruptionException.java @@ -0,0 +1,7 @@ +package ai.timefold.solver.core.impl.solver.exception; + +public class ScoreCorruptionException extends IllegalStateException { + public ScoreCorruptionException(String message) { + super(message); + } +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/UndoScoreCorruptionException.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/UndoScoreCorruptionException.java index 43e587cc955..0d354cce6dc 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/UndoScoreCorruptionException.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/UndoScoreCorruptionException.java @@ -1,12 +1,16 @@ package ai.timefold.solver.core.impl.solver.exception; +import ai.timefold.solver.core.config.solver.EnvironmentMode; + /** - * An exception that is thrown in {@link ai.timefold.solver.core.config.solver.EnvironmentMode#TRACKED_FULL_ASSERT} when - * undo score corruption is detected. It contains the working solution before the move, after the move, and after the undo move, - * as well as the move that caused the corruption. You can catch this exception to create a reproducer of the corruption. + * An exception that is thrown in {@link EnvironmentMode#TRACKED_FULL_ASSERT} when undo score corruption is detected. + * It contains the working solution before the move, after the move, and after the undo move, + * as well as the move that caused the corruption. + * You can catch this exception to create a reproducer of the corruption. * The API for this exception is currently unstable. */ -public class UndoScoreCorruptionException extends IllegalStateException { +public final class UndoScoreCorruptionException extends ScoreCorruptionException { + private final Object beforeMoveSolution; private final Object afterMoveSolution; private final Object afterUndoSolution; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/VariableCorruptionException.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/VariableCorruptionException.java new file mode 100644 index 00000000000..e4216dc1fee --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/VariableCorruptionException.java @@ -0,0 +1,7 @@ +package ai.timefold.solver.core.impl.solver.exception; + +public final class VariableCorruptionException extends IllegalStateException { + public VariableCorruptionException(String message) { + super(message); + } +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/package-info.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/package-info.java new file mode 100644 index 00000000000..c8cde550122 --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/exception/package-info.java @@ -0,0 +1,6 @@ +/** + * Contains various exceptions thrown by the solver. + * Their API is currently unstable, including their FQN. + * Catch these exceptions at your own risk. + */ +package ai.timefold.solver.core.impl.solver.exception; \ No newline at end of file diff --git a/core/src/main/resources/solver.xsd b/core/src/main/resources/solver.xsd index 868923ef66a..9056cceb8e6 100644 --- a/core/src/main/resources/solver.xsd +++ b/core/src/main/resources/solver.xsd @@ -1569,6 +1569,8 @@ + + diff --git a/core/src/test/java/ai/timefold/solver/core/api/solver/SolverManagerTest.java b/core/src/test/java/ai/timefold/solver/core/api/solver/SolverManagerTest.java index 53c7e01af59..41282414cbd 100644 --- a/core/src/test/java/ai/timefold/solver/core/api/solver/SolverManagerTest.java +++ b/core/src/test/java/ai/timefold/solver/core/api/solver/SolverManagerTest.java @@ -42,6 +42,7 @@ import ai.timefold.solver.core.config.localsearch.LocalSearchPhaseConfig; import ai.timefold.solver.core.config.phase.PhaseConfig; import ai.timefold.solver.core.config.phase.custom.CustomPhaseConfig; +import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.config.solver.SolverConfig; import ai.timefold.solver.core.config.solver.SolverManagerConfig; import ai.timefold.solver.core.config.solver.termination.TerminationConfig; @@ -591,6 +592,7 @@ void testScoreCalculationCountForFinishedJob() throws ExecutionException, Interr var terminationConfig = new TerminationConfig() .withScoreCalculationCountLimit(5L); var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class) + .withEnvironmentMode(EnvironmentMode.REPRODUCIBLE_UNGUARDED) .withTerminationConfig(terminationConfig); try (var solverManager = createDefaultSolverManager(solverConfig)) { diff --git a/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java b/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java index 499fe5ee458..607be72b1d8 100644 --- a/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java +++ b/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java @@ -84,16 +84,15 @@ void determinism(EnvironmentMode environmentMode) { Solver solver2 = SolverFactory. create(solverConfig).buildSolver(); switch (environmentMode) { - case NON_REPRODUCIBLE -> { - assertNonReproducibility(solver1, solver2); - } + case NON_REPRODUCIBLE -> assertNonReproducibility(solver1, solver2); case TRACKED_FULL_ASSERT, FULL_ASSERT, FAST_ASSERT, NON_INTRUSIVE_FULL_ASSERT, - REPRODUCIBLE -> { + REPRODUCIBLE_UNGUARDED, + REPRODUCIBLE -> assertReproducibility(solver1, solver2); - } + default -> throw new IllegalStateException("Unexpected value: " + environmentMode); } } @@ -131,8 +130,7 @@ void corruptedUndoShadowVariableListener(EnvironmentMode environmentMode) { "Variables that are different between before and undo", "Actual value (v2) of variable valueClone on CorruptedUndoShadowEntity entity (CorruptedUndoShadowEntity) differs from expected (v1)"); } - case FULL_ASSERT, - FAST_ASSERT -> { + case FULL_ASSERT, FAST_ASSERT -> { // FAST_ASSERT does not create snapshots since it is not intrusive, and hence it can only // detect the undo corruption and not what caused it var e1 = new CorruptedUndoShadowEntity("e1"); @@ -153,11 +151,10 @@ void corruptedUndoShadowVariableListener(EnvironmentMode environmentMode) { List.of(v1, v2)))) .withMessageContainingAll("corrupted undoMove"); } - case REPRODUCIBLE, - NON_REPRODUCIBLE, - NON_INTRUSIVE_FULL_ASSERT -> { + case REPRODUCIBLE, REPRODUCIBLE_UNGUARDED, NON_REPRODUCIBLE, NON_INTRUSIVE_FULL_ASSERT -> { // No exception expected } + default -> throw new IllegalStateException("Unexpected value: " + environmentMode); } } @@ -169,24 +166,13 @@ void corruptedConstraints(EnvironmentMode environmentMode) { setSolverConfigCalculatorClass(solverConfig, TestdataCorruptedDifferentValuesCalculator.class); switch (environmentMode) { - case TRACKED_FULL_ASSERT -> { - assertIllegalStateExceptionWhileSolving( - solverConfig, - "not the uncorruptedScore"); - } - case FULL_ASSERT, - NON_INTRUSIVE_FULL_ASSERT -> { - assertIllegalStateExceptionWhileSolving( - solverConfig, - "not the uncorruptedScore"); - } - case FAST_ASSERT -> { - assertIllegalStateExceptionWhileSolving( - solverConfig, - "Score corruption analysis could not be generated "); - } - case REPRODUCIBLE, - NON_REPRODUCIBLE -> { + case TRACKED_FULL_ASSERT -> + assertIllegalStateExceptionWhileSolving(solverConfig, "not the uncorruptedScore"); + case FULL_ASSERT, NON_INTRUSIVE_FULL_ASSERT -> + assertIllegalStateExceptionWhileSolving(solverConfig, "not the uncorruptedScore"); + case FAST_ASSERT -> + assertIllegalStateExceptionWhileSolving(solverConfig, "Score corruption analysis could not be generated "); + case REPRODUCIBLE, REPRODUCIBLE_UNGUARDED, NON_REPRODUCIBLE -> { // No exception expected } } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index 8f83a4fdcce..e6ebf13e8f4 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -127,6 +127,36 @@ void solve() { assertThat(solution.getScore().isSolutionInitialized()).isTrue(); } + @Test + void solveCorruptedEasyGuarded() { + var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class) + .withEnvironmentMode(EnvironmentMode.REPRODUCIBLE) + .withEasyScoreCalculatorClass(CorruptedEasyScoreCalculator.class); + + var solution = new TestdataSolution("s1"); + solution.setValueList(Arrays.asList(new TestdataValue("v1"), new TestdataValue("v2"))); + solution.setEntityList(Arrays.asList(new TestdataEntity("e1"), new TestdataEntity("e2"))); + + Assertions.assertThatThrownBy(() -> PlannerTestUtils.solve(solverConfig, solution, false)) + .hasMessageContaining("corruption") + .hasMessageContaining(EnvironmentMode.FULL_ASSERT.name()) + .hasMessageContaining(EnvironmentMode.REPRODUCIBLE_UNGUARDED.name()); + } + + @Test + void solveCorruptedEasyUnguarded() { + var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class) + .withEnvironmentMode(EnvironmentMode.REPRODUCIBLE_UNGUARDED) + .withEasyScoreCalculatorClass(CorruptedEasyScoreCalculator.class); + + var solution = new TestdataSolution("s1"); + solution.setValueList(Arrays.asList(new TestdataValue("v1"), new TestdataValue("v2"))); + solution.setEntityList(Arrays.asList(new TestdataEntity("e1"), new TestdataEntity("e2"))); + + Assertions.assertThatNoException() + .isThrownBy(() -> PlannerTestUtils.solve(solverConfig, solution, true)); + } + @Test void solveCorruptedEasyUninitialized() { var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class) From 76afd3408fa1dfe11abb53b4a1f3e810102f342e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Mon, 24 Feb 2025 16:47:27 +0100 Subject: [PATCH 02/14] Naming --- .../impl/report/BenchmarkReport.java | 5 +- benchmark/src/main/resources/benchmark.xsd | 8 +- .../core/config/solver/EnvironmentMode.java | 103 ++++++++++++------ .../core/config/solver/SolverConfig.java | 2 +- .../impl/bavet/common/AbstractGroupNode.java | 2 +- .../decider/ConstructionHeuristicDecider.java | 2 +- .../DefaultExhaustiveSearchPhase.java | 2 +- .../DefaultExhaustiveSearchPhaseFactory.java | 2 +- .../move/factory/MoveIteratorFactory.java | 2 +- .../decider/LocalSearchDecider.java | 2 +- .../solver/core/impl/phase/AbstractPhase.java | 8 +- .../director/ScoreDirectorFactoryFactory.java | 4 +- .../impl/solver/DefaultSolverFactory.java | 7 +- core/src/main/resources/solver.xsd | 6 +- .../core/api/solver/SolverManagerTest.java | 2 +- .../config/solver/EnvironmentModeTest.java | 16 +-- .../ConstraintWeightOverridesTest.java | 2 +- .../HeuristicConfigPolicyTestUtils.java | 2 +- .../ScoreDirectorFactoryFactoryTest.java | 6 +- .../easy/EasyScoreDirectorSemanticsTest.java | 8 +- ...IncrementalScoreDirectorSemanticsTest.java | 6 +- ...treamsBavetScoreDirectorSemanticsTest.java | 6 +- .../BavetConstraintStreamImplSupport.java | 4 +- .../core/impl/solver/DefaultSolverTest.java | 6 +- .../pages/constraints-and-score/overview.adoc | 4 +- .../optimization-algorithms/overview.adoc | 4 +- .../hello-world/hello-world-quickstart.adoc | 6 +- .../quarkus-vehicle-routing-quickstart.adoc | 6 +- .../quarkus/quarkus-quickstart.adoc | 2 +- .../shared/try-the-application.adoc | 4 +- .../spring-boot/spring-boot-quickstart.adoc | 6 +- .../benchmarking-and-tweaking.adoc | 2 +- .../running-the-solver.adoc | 29 ++--- .../src/main/python/config/_config.py | 26 +++-- ...dProcessorMultipleSolversEmptyAppTest.java | 2 +- ...ipleSolversInvalidConstraintClassTest.java | 14 +-- ...MultipleSolversInvalidEntityClassTest.java | 2 +- ...ltipleSolversInvalidSolutionClassTest.java | 4 +- ...ocessorMultipleSolversXMLPropertyTest.java | 2 +- .../quarkus/config/SolverRuntimeConfig.java | 2 +- .../config/SolverProperties.java | 2 +- 41 files changed, 192 insertions(+), 138 deletions(-) diff --git a/benchmark/src/main/java/ai/timefold/solver/benchmark/impl/report/BenchmarkReport.java b/benchmark/src/main/java/ai/timefold/solver/benchmark/impl/report/BenchmarkReport.java index 41dd0c00514..6ec2c0615ef 100644 --- a/benchmark/src/main/java/ai/timefold/solver/benchmark/impl/report/BenchmarkReport.java +++ b/benchmark/src/main/java/ai/timefold/solver/benchmark/impl/report/BenchmarkReport.java @@ -316,10 +316,11 @@ public List getWarningList() { + " Maybe reduce the parallelBenchmarkCount."); } EnvironmentMode environmentMode = plannerBenchmarkResult.getEnvironmentMode(); - if (environmentMode != null && environmentMode.isAsserted()) { + if (environmentMode != null && environmentMode.isStepAssertOrMore()) { + // Phase assert performance impact is negligible. warningList.add("The environmentMode (" + environmentMode + ") is asserting." + " This decreases performance." - + " Maybe set the environmentMode to " + EnvironmentMode.REPRODUCIBLE + "."); + + " Maybe set the environmentMode to " + EnvironmentMode.PHASE_ASSERT + "."); } LoggingLevel loggingLevelTimefoldCore = plannerBenchmarkResult.getLoggingLevelTimefoldSolverCore(); if (loggingLevelTimefoldCore == LoggingLevel.TRACE) { diff --git a/benchmark/src/main/resources/benchmark.xsd b/benchmark/src/main/resources/benchmark.xsd index cc4a93a888c..16baee7d16c 100644 --- a/benchmark/src/main/resources/benchmark.xsd +++ b/benchmark/src/main/resources/benchmark.xsd @@ -2627,13 +2627,19 @@ + + + + + + - + diff --git a/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java b/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java index 3d2c63ad159..e967d84bb16 100644 --- a/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java +++ b/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java @@ -1,9 +1,12 @@ package ai.timefold.solver.core.config.solver; +import java.util.HashMap; +import java.util.HashSet; import java.util.Random; import jakarta.xml.bind.annotation.XmlEnum; +import ai.timefold.solver.core.api.domain.entity.PlanningEntity; import ai.timefold.solver.core.api.solver.Solver; import ai.timefold.solver.core.impl.heuristic.move.Move; import ai.timefold.solver.core.impl.score.director.InnerScoreDirector; @@ -27,7 +30,7 @@ public enum EnvironmentMode { * Because it tracks genuine and shadow variables, it is able to report precisely what variables caused the corruption and * report any missed {@link ai.timefold.solver.core.api.domain.variable.VariableListener} events. *

- * This mode is reproducible (see {@link #REPRODUCIBLE} mode). + * This mode is reproducible (see {@link #PHASE_ASSERT} mode). *

* This mode is intrusive because it calls the {@link InnerScoreDirector#calculateScore()} more frequently than a non assert * mode. @@ -35,13 +38,12 @@ public enum EnvironmentMode { * This mode is by far the slowest of all the modes. */ TRACKED_FULL_ASSERT(true), - /** * This mode turns on all assertions * to fail-fast on a bug in a {@link Move} implementation, a constraint, the engine itself or something else * at a horrible performance cost. *

- * This mode is reproducible (see {@link #REPRODUCIBLE} mode). + * This mode is reproducible (see {@link #PHASE_ASSERT} mode). *

* This mode is intrusive because it calls the {@link InnerScoreDirector#calculateScore()} more frequently * than a non assert mode. @@ -54,50 +56,64 @@ public enum EnvironmentMode { * to fail-fast on a bug in a {@link Move} implementation, a constraint, the engine itself or something else * at an overwhelming performance cost. *

- * This mode is reproducible (see {@link #REPRODUCIBLE} mode). + * This mode is reproducible (see {@link #PHASE_ASSERT} mode). *

- * This mode is non-intrusive, unlike {@link #FULL_ASSERT} and {@link #FAST_ASSERT}. + * This mode is non-intrusive, unlike {@link #FULL_ASSERT} and {@link #STEP_ASSERT}. *

* This mode is horribly slow. */ NON_INTRUSIVE_FULL_ASSERT(true), /** - * This mode turns on several assertions (but not all of them) - * to fail-fast on a bug in a {@link Move} implementation, a constraint rule, the engine itself or something else + * This mode turns on several assertions to fail-fast + * on a bug in a {@link Move} implementation, a constraint rule, the engine itself or something else * at a reasonable performance cost (in development at least). *

- * This mode is reproducible (see {@link #REPRODUCIBLE} mode). + * This mode is reproducible (see {@link #PHASE_ASSERT} mode). *

* This mode is intrusive because it calls the {@link InnerScoreDirector#calculateScore()} more frequently - * than a non assert mode. + * than a non-assert mode. *

* This mode is slow. */ + STEP_ASSERT(true), + /** + * @deprecated Prefer {@link #STEP_ASSERT}. + */ + @Deprecated(forRemoval = true, since = "1.20.0") FAST_ASSERT(true), /** - * The reproducible mode is the default mode because it is recommended during development. - * In this mode, 2 runs on the same computer will execute the same code in the same order. + * This is the default mode as it is recommended during development, + * and runs minimal correctness checks that serve to quickly identify score corruption bugs. + *

+ * In this mode, two runs on the same computer will execute the same code in the same order. * They will also yield the same result, except if they use a time based termination * and they have a sufficiently large difference in allocated CPU time. * This allows you to benchmark new optimizations (such as a new {@link Move} implementation) fairly * and reproduce bugs in your code reliably. *

- * Warning: some code can disrupt reproducibility regardless of this mode. See the reference manual for more info. + * Warning: some code can disrupt reproducibility regardless of this mode. + * This typically happens when user code serves data such as {@link PlanningEntity planning entities} + * from collections without defined iteration order, such as {@link HashSet} or {@link HashMap}. *

* In practice, this mode uses the default random seed, - * and it also disables certain concurrency optimizations (such as work stealing). + * and it also disables certain concurrency optimizations, such as work stealing. */ + PHASE_ASSERT(true), + /** + * @deprecated Prefer {@link #NO_ASSERT}. + */ + @Deprecated(forRemoval = true, since = "1.20.0") REPRODUCIBLE(false), /** - * As defined by {@link #REPRODUCIBLE}, but it also disables certain bug detection mechanisms. - * This mode will run marginally faster than {@link #REPRODUCIBLE}, + * As defined by {@link #PHASE_ASSERT}, but disables every single bug detection mechanism. + * This mode will run negligibly faster than {@link #PHASE_ASSERT}, * but will allow some bugs in user code (such as score corruptions) to go unnoticed. - * Use this mode during development when you are confident that your code is bug-free, + * Use this mode when you are confident that your code is bug-free, * or when you want to ignore a known bug temporarily. */ - REPRODUCIBLE_UNGUARDED(false, false), + NO_ASSERT(false), /** - * The non-reproducible mode is equally fast or slightly faster than {@link #REPRODUCIBLE_UNGUARDED}. + * The non-reproducible mode is equally fast or slightly faster than {@link #NO_ASSERT}. *

* The random seed is different on every run, which makes it more robust against an unlucky random seed. * An unlucky random seed gives a bad result on a certain data set with a certain solver configuration. @@ -106,22 +122,22 @@ public enum EnvironmentMode { *

* In multithreaded scenarios, this mode allows the use of work stealing and other non-deterministic speed tricks. */ - NON_REPRODUCIBLE(false, false); + NON_REPRODUCIBLE(false); private final boolean asserted; - private final boolean guarded; EnvironmentMode(boolean asserted) { - this(asserted, true); - } - - EnvironmentMode(boolean asserted, boolean guarded) { this.asserted = asserted; - this.guarded = guarded; } - public boolean isGuarded() { - return guarded; + public boolean isStepAssertOrMore() { + return switch (this) { + case NON_REPRODUCIBLE, NO_ASSERT, PHASE_ASSERT, STEP_ASSERT, NON_INTRUSIVE_FULL_ASSERT, FULL_ASSERT, + TRACKED_FULL_ASSERT -> + isAsserted() && this != PHASE_ASSERT; + case REPRODUCIBLE -> PHASE_ASSERT.isAsserted(); + case FAST_ASSERT -> STEP_ASSERT.isAsserted(); + }; } public boolean isAsserted() { @@ -129,17 +145,31 @@ public boolean isAsserted() { } public boolean isNonIntrusiveFullAsserted() { - if (!isAsserted()) { - return false; - } - return this != FAST_ASSERT; + return switch (this) { + case NON_REPRODUCIBLE, NO_ASSERT, PHASE_ASSERT, STEP_ASSERT, NON_INTRUSIVE_FULL_ASSERT, FULL_ASSERT, + TRACKED_FULL_ASSERT -> { + if (!isStepAssertOrMore()) { + yield false; + } + yield this != STEP_ASSERT; + } + case REPRODUCIBLE -> PHASE_ASSERT.isNonIntrusiveFullAsserted(); + case FAST_ASSERT -> STEP_ASSERT.isNonIntrusiveFullAsserted(); + }; } - public boolean isIntrusiveFastAsserted() { - if (!isAsserted()) { - return false; - } - return this != NON_INTRUSIVE_FULL_ASSERT; + public boolean isIntrusiveStepAsserted() { + return switch (this) { + case NON_REPRODUCIBLE, NO_ASSERT, PHASE_ASSERT, STEP_ASSERT, NON_INTRUSIVE_FULL_ASSERT, FULL_ASSERT, + TRACKED_FULL_ASSERT -> { + if (!isStepAssertOrMore()) { + yield false; + } + yield this != NON_INTRUSIVE_FULL_ASSERT; + } + case REPRODUCIBLE -> PHASE_ASSERT.isIntrusiveStepAsserted(); + case FAST_ASSERT -> STEP_ASSERT.isIntrusiveStepAsserted(); + }; } public boolean isReproducible() { @@ -149,4 +179,5 @@ public boolean isReproducible() { public boolean isTracking() { return this == TRACKED_FULL_ASSERT; } + } diff --git a/core/src/main/java/ai/timefold/solver/core/config/solver/SolverConfig.java b/core/src/main/java/ai/timefold/solver/core/config/solver/SolverConfig.java index bc2c86b1384..8af8e335edf 100644 --- a/core/src/main/java/ai/timefold/solver/core/config/solver/SolverConfig.java +++ b/core/src/main/java/ai/timefold/solver/core/config/solver/SolverConfig.java @@ -633,7 +633,7 @@ public boolean canTerminate() { } public @NonNull EnvironmentMode determineEnvironmentMode() { - return Objects.requireNonNullElse(environmentMode, EnvironmentMode.REPRODUCIBLE); + return Objects.requireNonNullElse(environmentMode, EnvironmentMode.PHASE_ASSERT); } public @NonNull DomainAccessType determineDomainAccessType() { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/bavet/common/AbstractGroupNode.java b/core/src/main/java/ai/timefold/solver/core/impl/bavet/common/AbstractGroupNode.java index 464697be0a9..a9b1ec1f3d9 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/bavet/common/AbstractGroupNode.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/bavet/common/AbstractGroupNode.java @@ -81,7 +81,7 @@ protected AbstractGroupNode(int groupStoreIndex, int undoStoreIndex, updateOutTupleToFinisher(outTuple, group.getResultContainer()); } }) : new DynamicPropagationQueue<>(nextNodesTupleLifecycle); - this.useAssertingGroupKey = environmentMode.isAsserted(); + this.useAssertingGroupKey = environmentMode.isStepAssertOrMore(); } protected AbstractGroupNode(int groupStoreIndex, diff --git a/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/decider/ConstructionHeuristicDecider.java b/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/decider/ConstructionHeuristicDecider.java index da2ea01d00c..e266db05443 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/decider/ConstructionHeuristicDecider.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/decider/ConstructionHeuristicDecider.java @@ -51,7 +51,7 @@ public ConstructionHeuristicForager getForager() { public void enableAssertions(EnvironmentMode environmentMode) { this.assertMoveScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); - this.assertExpectedUndoMoveScore = environmentMode.isIntrusiveFastAsserted(); + this.assertExpectedUndoMoveScore = environmentMode.isIntrusiveStepAsserted(); } // ************************************************************************ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java index d8a74328318..c1058dfa036 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java @@ -249,7 +249,7 @@ public Builder(int phaseIndex, String logIndentation, Termination pha public Builder enableAssertions(EnvironmentMode environmentMode) { super.enableAssertions(environmentMode); assertWorkingSolutionScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); - assertExpectedWorkingSolutionScore = environmentMode.isIntrusiveFastAsserted(); + assertExpectedWorkingSolutionScore = environmentMode.isIntrusiveStepAsserted(); return this; } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java index ba929407c7a..769a7378ffb 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java @@ -146,7 +146,7 @@ private ExhaustiveSearchDecider buildDecider(HeuristicConfigPolicy scoreDirector) { * @param scoreDirector never null, the {@link ScoreDirector} * which has the {@link ScoreDirector#getWorkingSolution()} of which the {@link Move}s need to be generated * @param workingRandom never null, the {@link Random} to use when any random number is needed, - * so {@link EnvironmentMode#REPRODUCIBLE} works correctly + * so {@link EnvironmentMode#PHASE_ASSERT} works correctly * @return never null, an {@link Iterator} that is allowed (or even presumed) to be never ending * @throws UnsupportedOperationException if only {@link #createOriginalMoveIterator(ScoreDirector)} is supported */ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/LocalSearchDecider.java b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/LocalSearchDecider.java index aa72c0edd29..437bd85b4ff 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/LocalSearchDecider.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/LocalSearchDecider.java @@ -63,7 +63,7 @@ public LocalSearchForager getForager() { public void enableAssertions(EnvironmentMode environmentMode) { assertMoveScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); - assertExpectedUndoMoveScore = environmentMode.isIntrusiveFastAsserted(); + assertExpectedUndoMoveScore = environmentMode.isIntrusiveStepAsserted(); } // ************************************************************************ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java b/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java index 2ffe47a25ba..8a4e94c4054 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java @@ -142,7 +142,7 @@ public void phaseEnded(AbstractPhaseScope phaseScope) { and if you are convinced that the problem is not in your code, please report a bug to Timefold. At your own risk, you may run your solver with %s or %s instead to ignore this error.""" .formatted(EnvironmentMode.class.getSimpleName(), - EnvironmentMode.FULL_ASSERT, EnvironmentMode.REPRODUCIBLE_UNGUARDED, + EnvironmentMode.FULL_ASSERT, EnvironmentMode.NO_ASSERT, EnvironmentMode.NON_REPRODUCIBLE), e); } @@ -269,10 +269,10 @@ protected AbstractPhaseBuilder(int phaseIndex, String logIndentation, Terminatio } public AbstractPhaseBuilder enableAssertions(EnvironmentMode environmentMode) { - assertPhaseScoreFromScratch = environmentMode.isGuarded() && !environmentMode.isAsserted(); + assertPhaseScoreFromScratch = environmentMode.isAsserted(); assertStepScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); - assertExpectedStepScore = environmentMode.isIntrusiveFastAsserted(); - assertShadowVariablesAreNotStaleAfterStep = environmentMode.isIntrusiveFastAsserted(); + assertExpectedStepScore = environmentMode.isIntrusiveStepAsserted(); + assertShadowVariablesAreNotStaleAfterStep = environmentMode.isIntrusiveStepAsserted(); return this; } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactory.java index b3423d75d4c..a2158f8fcb2 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactory.java @@ -32,10 +32,10 @@ public InnerScoreDirectorFactory buildScoreDirectorFactory(En .formatted(assertionScoreDirectorFactory, assertionScoreDirectorFactory.getAssertionScoreDirectorFactory())); } - if (environmentMode.compareTo(EnvironmentMode.FAST_ASSERT) > 0) { + if (environmentMode.compareTo(EnvironmentMode.STEP_ASSERT) > 0) { throw new IllegalArgumentException( "A non-null assertionScoreDirectorFactory (%s) requires an environmentMode (%s) of %s or lower." - .formatted(assertionScoreDirectorFactory, environmentMode, EnvironmentMode.FAST_ASSERT)); + .formatted(assertionScoreDirectorFactory, environmentMode, EnvironmentMode.STEP_ASSERT)); } var assertionScoreDirectorFactoryFactory = new ScoreDirectorFactoryFactory(assertionScoreDirectorFactory); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolverFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolverFactory.java index 23344614480..42589849c48 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolverFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolverFactory.java @@ -100,8 +100,9 @@ public > InnerScoreDirectorFactory buildSolutionDescriptor() { solverConfig.getGizmoSolutionClonerMap(), solverConfig.getEntityClassList()); EnvironmentMode environmentMode = solverConfig.determineEnvironmentMode(); - if (environmentMode.isAsserted()) { + if (environmentMode.isStepAssertOrMore()) { solutionDescriptor.setAssertModelForCloning(true); } diff --git a/core/src/main/resources/solver.xsd b/core/src/main/resources/solver.xsd index 9056cceb8e6..cab8715c17f 100644 --- a/core/src/main/resources/solver.xsd +++ b/core/src/main/resources/solver.xsd @@ -1565,11 +1565,15 @@ + + + + - + diff --git a/core/src/test/java/ai/timefold/solver/core/api/solver/SolverManagerTest.java b/core/src/test/java/ai/timefold/solver/core/api/solver/SolverManagerTest.java index 41282414cbd..232582f63df 100644 --- a/core/src/test/java/ai/timefold/solver/core/api/solver/SolverManagerTest.java +++ b/core/src/test/java/ai/timefold/solver/core/api/solver/SolverManagerTest.java @@ -592,7 +592,7 @@ void testScoreCalculationCountForFinishedJob() throws ExecutionException, Interr var terminationConfig = new TerminationConfig() .withScoreCalculationCountLimit(5L); var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class) - .withEnvironmentMode(EnvironmentMode.REPRODUCIBLE_UNGUARDED) + .withEnvironmentMode(EnvironmentMode.NO_ASSERT) .withTerminationConfig(terminationConfig); try (var solverManager = createDefaultSolverManager(solverConfig)) { diff --git a/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java b/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java index 607be72b1d8..3a7816f01f1 100644 --- a/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java +++ b/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java @@ -87,9 +87,11 @@ void determinism(EnvironmentMode environmentMode) { case NON_REPRODUCIBLE -> assertNonReproducibility(solver1, solver2); case TRACKED_FULL_ASSERT, FULL_ASSERT, - FAST_ASSERT, + STEP_ASSERT, NON_INTRUSIVE_FULL_ASSERT, - REPRODUCIBLE_UNGUARDED, + NO_ASSERT, + PHASE_ASSERT, + FAST_ASSERT, REPRODUCIBLE -> assertReproducibility(solver1, solver2); default -> throw new IllegalStateException("Unexpected value: " + environmentMode); @@ -130,8 +132,8 @@ void corruptedUndoShadowVariableListener(EnvironmentMode environmentMode) { "Variables that are different between before and undo", "Actual value (v2) of variable valueClone on CorruptedUndoShadowEntity entity (CorruptedUndoShadowEntity) differs from expected (v1)"); } - case FULL_ASSERT, FAST_ASSERT -> { - // FAST_ASSERT does not create snapshots since it is not intrusive, and hence it can only + case FULL_ASSERT, STEP_ASSERT, FAST_ASSERT -> { + // STEP_ASSERT does not create snapshots since it is not intrusive, and hence it can only // detect the undo corruption and not what caused it var e1 = new CorruptedUndoShadowEntity("e1"); var e2 = new CorruptedUndoShadowEntity("e2"); @@ -151,7 +153,7 @@ void corruptedUndoShadowVariableListener(EnvironmentMode environmentMode) { List.of(v1, v2)))) .withMessageContainingAll("corrupted undoMove"); } - case REPRODUCIBLE, REPRODUCIBLE_UNGUARDED, NON_REPRODUCIBLE, NON_INTRUSIVE_FULL_ASSERT -> { + case PHASE_ASSERT, NO_ASSERT, NON_REPRODUCIBLE, NON_INTRUSIVE_FULL_ASSERT, REPRODUCIBLE -> { // No exception expected } default -> throw new IllegalStateException("Unexpected value: " + environmentMode); @@ -170,9 +172,9 @@ void corruptedConstraints(EnvironmentMode environmentMode) { assertIllegalStateExceptionWhileSolving(solverConfig, "not the uncorruptedScore"); case FULL_ASSERT, NON_INTRUSIVE_FULL_ASSERT -> assertIllegalStateExceptionWhileSolving(solverConfig, "not the uncorruptedScore"); - case FAST_ASSERT -> + case STEP_ASSERT -> assertIllegalStateExceptionWhileSolving(solverConfig, "Score corruption analysis could not be generated "); - case REPRODUCIBLE, REPRODUCIBLE_UNGUARDED, NON_REPRODUCIBLE -> { + case PHASE_ASSERT, NO_ASSERT, NON_REPRODUCIBLE -> { // No exception expected } } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/solution/ConstraintWeightOverridesTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/solution/ConstraintWeightOverridesTest.java index 1a302f747f7..b230050752c 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/solution/ConstraintWeightOverridesTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/solution/ConstraintWeightOverridesTest.java @@ -80,7 +80,7 @@ void appliesOverridesToConstraintProvider() { try (var scoreDirector = BavetConstraintStreamScoreDirectorFactory.buildScoreDirectorFactory(solutionDescriptor, new ScoreDirectorFactoryConfig() .withConstraintProviderClass(TestdataConstraintWeightOverridesConstraintProvider.class), - EnvironmentMode.REPRODUCIBLE) + EnvironmentMode.PHASE_ASSERT) .buildScoreDirector(false, ConstraintMatchPolicy.DISABLED)) { // Default weights scoreDirector.setWorkingSolution(solution); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicyTestUtils.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicyTestUtils.java index 8f2d0b162a6..e6990deb3e5 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicyTestUtils.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicyTestUtils.java @@ -23,7 +23,7 @@ public static HeuristicConfigPolicy buildHeuristicConfigPolicy buildHeuristicConfigPolicy(SolutionDescriptor solutionDescriptor, EntitySorterManner entitySorterManner) { return new HeuristicConfigPolicy.Builder() - .withEnvironmentMode(EnvironmentMode.REPRODUCIBLE) + .withEnvironmentMode(EnvironmentMode.PHASE_ASSERT) .withRandom(new Random()) .withSolutionDescriptor(solutionDescriptor) .withClassInstanceCache(ClassInstanceCache.create()) diff --git a/core/src/test/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactoryTest.java b/core/src/test/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactoryTest.java index 3c3e35c0798..31b3fe16e19 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactoryTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactoryTest.java @@ -55,7 +55,7 @@ void buildWithAssertionScoreDirectorFactory() { AbstractScoreDirectorFactory scoreDirectorFactory = (AbstractScoreDirectorFactory) buildTestdataScoreDirectoryFactory(config, - EnvironmentMode.FAST_ASSERT); + EnvironmentMode.STEP_ASSERT); ScoreDirectorFactory assertionScoreDirectorFactory = scoreDirectorFactory.getAssertionScoreDirectorFactory(); @@ -86,7 +86,7 @@ private > ScoreDirectorFactory bu } private ScoreDirectorFactory buildTestdataScoreDirectoryFactory(ScoreDirectorFactoryConfig config) { - return buildTestdataScoreDirectoryFactory(config, EnvironmentMode.REPRODUCIBLE); + return buildTestdataScoreDirectoryFactory(config, EnvironmentMode.PHASE_ASSERT); } @Test @@ -95,7 +95,7 @@ void constraintStreamsBavet() { .withConstraintProviderClass(TestdataConstraintProvider.class); var scoreDirectorFactory = BavetConstraintStreamScoreDirectorFactory.buildScoreDirectorFactory(TestdataSolution.buildSolutionDescriptor(), - config, EnvironmentMode.REPRODUCIBLE); + config, EnvironmentMode.PHASE_ASSERT); assertThat(scoreDirectorFactory).isInstanceOf(BavetConstraintStreamScoreDirectorFactory.class); } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirectorSemanticsTest.java b/core/src/test/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirectorSemanticsTest.java index 83919167724..384f07348d9 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirectorSemanticsTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirectorSemanticsTest.java @@ -36,7 +36,7 @@ final class EasyScoreDirectorSemanticsTest extends AbstractScoreDirectorSemantic .withEasyScoreCalculatorClass(TestdataConstraintWeightEasyScoreCalculator.class); ScoreDirectorFactoryFactory scoreDirectorFactoryFactory = new ScoreDirectorFactoryFactory<>(scoreDirectorFactoryConfig); - return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.REPRODUCIBLE, solutionDescriptor); + return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.PHASE_ASSERT, solutionDescriptor); } @Override @@ -47,7 +47,7 @@ final class EasyScoreDirectorSemanticsTest extends AbstractScoreDirectorSemantic .withEasyScoreCalculatorClass(TestdataPinnedListEasyScoreCalculator.class); var scoreDirectorFactoryFactory = new ScoreDirectorFactoryFactory(scoreDirectorFactoryConfig); - return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.REPRODUCIBLE, solutionDescriptor); + return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.PHASE_ASSERT, solutionDescriptor); } @Override @@ -58,7 +58,7 @@ final class EasyScoreDirectorSemanticsTest extends AbstractScoreDirectorSemantic .withEasyScoreCalculatorClass(TestdataPinnedWithIndexListEasyScoreCalculator.class); var scoreDirectorFactoryFactory = new ScoreDirectorFactoryFactory(scoreDirectorFactoryConfig); - return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.REPRODUCIBLE, solutionDescriptor); + return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.PHASE_ASSERT, solutionDescriptor); } @Test @@ -87,7 +87,7 @@ private > ScoreDirectorFactory bu } private ScoreDirectorFactory buildTestdataScoreDirectoryFactory(ScoreDirectorFactoryConfig config) { - return buildTestdataScoreDirectoryFactory(config, EnvironmentMode.REPRODUCIBLE); + return buildTestdataScoreDirectoryFactory(config, EnvironmentMode.PHASE_ASSERT); } public static class TestCustomPropertiesEasyScoreCalculator diff --git a/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorSemanticsTest.java b/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorSemanticsTest.java index 9dec5de3243..a3b72514a66 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorSemanticsTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorSemanticsTest.java @@ -24,7 +24,7 @@ final class IncrementalScoreDirectorSemanticsTest extends AbstractScoreDirectorS .withIncrementalScoreCalculatorClass(TestdataConstraintWeighIncrementalScoreCalculator.class); var scoreDirectorFactoryFactory = new ScoreDirectorFactoryFactory( scoreDirectorFactoryConfig); - return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.REPRODUCIBLE, solutionDescriptor); + return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.PHASE_ASSERT, solutionDescriptor); } @Override @@ -35,7 +35,7 @@ final class IncrementalScoreDirectorSemanticsTest extends AbstractScoreDirectorS .withIncrementalScoreCalculatorClass(TestdataPinnedListIncrementalScoreCalculator.class); var scoreDirectorFactoryFactory = new ScoreDirectorFactoryFactory(scoreDirectorFactoryConfig); - return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.REPRODUCIBLE, solutionDescriptor); + return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.PHASE_ASSERT, solutionDescriptor); } @Override @@ -46,7 +46,7 @@ final class IncrementalScoreDirectorSemanticsTest extends AbstractScoreDirectorS .withIncrementalScoreCalculatorClass(TestdataPinnedWithIndexListIncrementalScoreCalculator.class); var scoreDirectorFactoryFactory = new ScoreDirectorFactoryFactory(scoreDirectorFactoryConfig); - return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.REPRODUCIBLE, solutionDescriptor); + return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.PHASE_ASSERT, solutionDescriptor); } } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/score/director/stream/ConstraintStreamsBavetScoreDirectorSemanticsTest.java b/core/src/test/java/ai/timefold/solver/core/impl/score/director/stream/ConstraintStreamsBavetScoreDirectorSemanticsTest.java index 90aa25284b6..bac2851b536 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/score/director/stream/ConstraintStreamsBavetScoreDirectorSemanticsTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/score/director/stream/ConstraintStreamsBavetScoreDirectorSemanticsTest.java @@ -24,7 +24,7 @@ final class ConstraintStreamsBavetScoreDirectorSemanticsTest extends AbstractSco .withConstraintProviderClass(TestdataConstraintWeightConstraintProvider.class); ScoreDirectorFactoryFactory scoreDirectorFactoryFactory = new ScoreDirectorFactoryFactory<>(scoreDirectorFactoryConfig); - return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.REPRODUCIBLE, solutionDescriptor); + return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.PHASE_ASSERT, solutionDescriptor); } @Override @@ -35,7 +35,7 @@ final class ConstraintStreamsBavetScoreDirectorSemanticsTest extends AbstractSco .withConstraintProviderClass(TestdataPinnedListConstraintProvider.class); var scoreDirectorFactoryFactory = new ScoreDirectorFactoryFactory(scoreDirectorFactoryConfig); - return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.REPRODUCIBLE, solutionDescriptor); + return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.PHASE_ASSERT, solutionDescriptor); } @Override @@ -46,7 +46,7 @@ final class ConstraintStreamsBavetScoreDirectorSemanticsTest extends AbstractSco .withConstraintProviderClass(TestdataPinnedWithIndexListConstraintProvider.class); var scoreDirectorFactoryFactory = new ScoreDirectorFactoryFactory(scoreDirectorFactoryConfig); - return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.REPRODUCIBLE, solutionDescriptor); + return scoreDirectorFactoryFactory.buildScoreDirectorFactory(EnvironmentMode.PHASE_ASSERT, solutionDescriptor); } } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetConstraintStreamImplSupport.java b/core/src/test/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetConstraintStreamImplSupport.java index 796ed1119f5..b851a9b6861 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetConstraintStreamImplSupport.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetConstraintStreamImplSupport.java @@ -18,12 +18,12 @@ public record BavetConstraintStreamImplSupport(ConstraintMatchPolicy constraintM public , Solution_> InnerScoreDirector buildScoreDirector( SolutionDescriptor solutionDescriptorSupplier, ConstraintProvider constraintProvider) { return (InnerScoreDirector) new BavetConstraintStreamScoreDirectorFactory<>( - solutionDescriptorSupplier, constraintProvider, EnvironmentMode.REPRODUCIBLE) + solutionDescriptorSupplier, constraintProvider, EnvironmentMode.PHASE_ASSERT) .buildScoreDirector(false, constraintMatchPolicy); } @Override public ConstraintFactory buildConstraintFactory(SolutionDescriptor solutionDescriptorSupplier) { - return new BavetConstraintFactory<>(solutionDescriptorSupplier, EnvironmentMode.REPRODUCIBLE); + return new BavetConstraintFactory<>(solutionDescriptorSupplier, EnvironmentMode.PHASE_ASSERT); } } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index e6ebf13e8f4..3521bdaef92 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -130,7 +130,7 @@ void solve() { @Test void solveCorruptedEasyGuarded() { var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class) - .withEnvironmentMode(EnvironmentMode.REPRODUCIBLE) + .withEnvironmentMode(EnvironmentMode.PHASE_ASSERT) .withEasyScoreCalculatorClass(CorruptedEasyScoreCalculator.class); var solution = new TestdataSolution("s1"); @@ -140,13 +140,13 @@ void solveCorruptedEasyGuarded() { Assertions.assertThatThrownBy(() -> PlannerTestUtils.solve(solverConfig, solution, false)) .hasMessageContaining("corruption") .hasMessageContaining(EnvironmentMode.FULL_ASSERT.name()) - .hasMessageContaining(EnvironmentMode.REPRODUCIBLE_UNGUARDED.name()); + .hasMessageContaining(EnvironmentMode.NO_ASSERT.name()); } @Test void solveCorruptedEasyUnguarded() { var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class) - .withEnvironmentMode(EnvironmentMode.REPRODUCIBLE_UNGUARDED) + .withEnvironmentMode(EnvironmentMode.NO_ASSERT) .withEasyScoreCalculatorClass(CorruptedEasyScoreCalculator.class); var solution = new TestdataSolution("s1"); diff --git a/docs/src/modules/ROOT/pages/constraints-and-score/overview.adoc b/docs/src/modules/ROOT/pages/constraints-and-score/overview.adoc index 39acab5d7d1..7615624d3bc 100644 --- a/docs/src/modules/ROOT/pages/constraints-and-score/overview.adoc +++ b/docs/src/modules/ROOT/pages/constraints-and-score/overview.adoc @@ -563,7 +563,7 @@ Alternatively, you can also specify the trend for each score level separately: [#invalidScoreDetection] === Invalid score detection -When you put the xref:using-timefold-solver/running-the-solver.adoc#environmentMode[`environmentMode`] in `FULL_ASSERT` (or ``FAST_ASSERT``), +When you put the xref:using-timefold-solver/running-the-solver.adoc#environmentMode[`environmentMode`] in `FULL_ASSERT` (or ``STEP_ASSERT``), it will detect score corruption in the xref:constraints-and-score/performance.adoc#incrementalScoreCalculationPerformance[incremental score calculation]. However, that will not verify that your score calculator actually implements your score constraints as your business desires. For example, one constraint might consistently match the wrong pattern. @@ -571,7 +571,7 @@ To verify the constraints against an independent implementation, configure a ``a [source,xml,options="nowrap"] ---- - FAST_ASSERT + STEP_ASSERT ... ...ConstraintProvider diff --git a/docs/src/modules/ROOT/pages/optimization-algorithms/overview.adoc b/docs/src/modules/ROOT/pages/optimization-algorithms/overview.adoc index 0e46c5f0bc1..e6126b884fb 100644 --- a/docs/src/modules/ROOT/pages/optimization-algorithms/overview.adoc +++ b/docs/src/modules/ROOT/pages/optimization-algorithms/overview.adoc @@ -377,7 +377,7 @@ Or use a combination that sums up to 150 minutes: [NOTE] ==== -This `Termination` will most likely sacrifice perfect reproducibility (even with `environmentMode` `REPRODUCIBLE`) +This `Termination` will most likely sacrifice perfect reproducibility (even with `environmentMode` `NO_ASSERT`) because the available CPU time differs frequently between runs: * The available CPU time influences the number of steps that can be taken, which might be a few more or less. @@ -472,7 +472,7 @@ the termination will only trigger if the local search phase does not improve the [NOTE] ==== -This `Termination` will most likely sacrifice perfect reproducibility (even with `environmentMode` ``REPRODUCIBLE``) +This `Termination` will most likely sacrifice perfect reproducibility (even with `environmentMode` ``NO_ASSERT``) as the available CPU time differs frequently between runs: * The available CPU time influences the number of steps that can be taken, which might be a few more or less. diff --git a/docs/src/modules/ROOT/pages/quickstart/hello-world/hello-world-quickstart.adoc b/docs/src/modules/ROOT/pages/quickstart/hello-world/hello-world-quickstart.adoc index 97802cbeec8..d0de7ad9852 100644 --- a/docs/src/modules/ROOT/pages/quickstart/hello-world/hello-world-quickstart.adoc +++ b/docs/src/modules/ROOT/pages/quickstart/hello-world/hello-world-quickstart.adoc @@ -996,10 +996,10 @@ The `info` log shows what Timefold Solver did in those five seconds: [source,options="nowrap"] ---- -... Solving started: time spent (33), best score (-8init/0hard/0soft), environment mode (REPRODUCIBLE), random (JDK with seed 0). +... Solving started: time spent (33), best score (-8init/0hard/0soft), environment mode (PHASE_ASSERT), random (JDK with seed 0). ... Construction Heuristic phase (0) ended: time spent (73), best score (0hard/0soft), move evaluation speed (459/sec), step total (4). ... Local Search phase (1) ended: time spent (5000), best score (0hard/0soft), move evaluation speed (28949/sec), step total (28398). -... Solving ended: time spent (5000), best score (0hard/0soft), move evaluation speed (28524/sec), phase total (2), environment mode (REPRODUCIBLE). +... Solving ended: time spent (5000), best score (0hard/0soft), move evaluation speed (28524/sec), phase total (2), environment mode (PHASE_ASSERT). ---- === Test the application @@ -1213,7 +1213,7 @@ Use `debug` logging to show every _step_: [source,options="nowrap"] ---- -... Solving started: time spent (67), best score (-20init/0hard/0soft), environment mode (REPRODUCIBLE), random (JDK with seed 0). +... Solving started: time spent (67), best score (-20init/0hard/0soft), environment mode (PHASE_ASSERT), random (JDK with seed 0). ... CH step (0), time spent (128), score (-18init/0hard/0soft), selected move count (15), picked move ([Math(101) {null -> Room A}, Math(101) {null -> MONDAY 08:30}]). ... CH step (1), time spent (145), score (-16init/0hard/0soft), selected move count (15), picked move ([Physics(102) {null -> Room A}, Physics(102) {null -> MONDAY 09:30}]). ... diff --git a/docs/src/modules/ROOT/pages/quickstart/quarkus-vehicle-routing/quarkus-vehicle-routing-quickstart.adoc b/docs/src/modules/ROOT/pages/quickstart/quarkus-vehicle-routing/quarkus-vehicle-routing-quickstart.adoc index d0654897f9f..dca7c83746b 100644 --- a/docs/src/modules/ROOT/pages/quickstart/quarkus-vehicle-routing/quarkus-vehicle-routing-quickstart.adoc +++ b/docs/src/modules/ROOT/pages/quickstart/quarkus-vehicle-routing/quarkus-vehicle-routing-quickstart.adoc @@ -509,10 +509,10 @@ On the server side, the `info` log shows what Timefold Solver did in those five [source,options="nowrap"] ---- -... Solving started: time spent (17), best score (-5init/0hard/0soft), environment mode (REPRODUCIBLE), move thread count (NONE), random (JDK with seed 0). +... Solving started: time spent (17), best score (-5init/0hard/0soft), environment mode (PHASE_ASSERT), move thread count (NONE), random (JDK with seed 0). ... Construction Heuristic phase (0) ended: time spent (33), best score (0hard/-18755soft), move evaluation speed (2222/sec), step total (5). ... Local Search phase (1) ended: time spent (5000), best score (0hard/-18716soft), move evaluation speed (89685/sec), step total (40343). -... Solving ended: time spent (5000), best score (0hard/-18716soft), move evaluation speed (89079/sec), phase total (2), environment mode (REPRODUCIBLE), move thread count (NONE). +... Solving ended: time spent (5000), best score (0hard/-18716soft), move evaluation speed (89079/sec), phase total (2), environment mode (PHASE_ASSERT), move thread count (NONE). ---- === Test the application @@ -908,7 +908,7 @@ Use `debug` logging to show every _step_: [source,options="nowrap"] ---- -... Solving started: time spent (67), best score (-20init/0hard/0soft), environment mode (REPRODUCIBLE), random (JDK with seed 0). +... Solving started: time spent (67), best score (-20init/0hard/0soft), environment mode (PHASE_ASSERT), random (JDK with seed 0). ... CH step (0), time spent (128), score (-18init/0hard/0soft), selected move count (15), picked move ([Math(101) {null -> Room A}, Math(101) {null -> MONDAY 08:30}]). ... CH step (1), time spent (145), score (-16init/0hard/0soft), selected move count (15), picked move ([Physics(102) {null -> Room A}, Physics(102) {null -> MONDAY 09:30}]). ... diff --git a/docs/src/modules/ROOT/pages/quickstart/quarkus/quarkus-quickstart.adoc b/docs/src/modules/ROOT/pages/quickstart/quarkus/quarkus-quickstart.adoc index cede4976dda..d4afc95e8c5 100644 --- a/docs/src/modules/ROOT/pages/quickstart/quarkus/quarkus-quickstart.adoc +++ b/docs/src/modules/ROOT/pages/quickstart/quarkus/quarkus-quickstart.adoc @@ -430,7 +430,7 @@ Use `debug` logging to show every _step_: [source,options="nowrap"] ---- -... Solving started: time spent (67), best score (-20init/0hard/0soft), environment mode (REPRODUCIBLE), random (JDK with seed 0). +... Solving started: time spent (67), best score (-20init/0hard/0soft), environment mode (PHASE_ASSERT), random (JDK with seed 0). ... CH step (0), time spent (128), score (-18init/0hard/0soft), selected move count (15), picked move ([Math(101) {null -> Room A}, Math(101) {null -> MONDAY 08:30}]). ... CH step (1), time spent (145), score (-16init/0hard/0soft), selected move count (15), picked move ([Physics(102) {null -> Room A}, Physics(102) {null -> MONDAY 09:30}]). ... diff --git a/docs/src/modules/ROOT/pages/quickstart/shared/try-the-application.adoc b/docs/src/modules/ROOT/pages/quickstart/shared/try-the-application.adoc index 54ace778ab6..99beabacdaf 100644 --- a/docs/src/modules/ROOT/pages/quickstart/shared/try-the-application.adoc +++ b/docs/src/modules/ROOT/pages/quickstart/shared/try-the-application.adoc @@ -29,8 +29,8 @@ On the server side, the `info` log shows what Timefold Solver did in those five [source,options="nowrap"] ---- -... Solving started: time spent (33), best score (-8init/0hard/0soft), environment mode (REPRODUCIBLE), random (JDK with seed 0). +... Solving started: time spent (33), best score (-8init/0hard/0soft), environment mode (PHASE_ASSERT), random (JDK with seed 0). ... Construction Heuristic phase (0) ended: time spent (73), best score (0hard/0soft), move evaluation speed (459/sec), step total (4). ... Local Search phase (1) ended: time spent (5000), best score (0hard/0soft), move evaluation speed (28949/sec), step total (28398). -... Solving ended: time spent (5000), best score (0hard/0soft), move evaluation speed (28524/sec), phase total (2), environment mode (REPRODUCIBLE). +... Solving ended: time spent (5000), best score (0hard/0soft), move evaluation speed (28524/sec), phase total (2), environment mode (PHASE_ASSERT). ---- diff --git a/docs/src/modules/ROOT/pages/quickstart/spring-boot/spring-boot-quickstart.adoc b/docs/src/modules/ROOT/pages/quickstart/spring-boot/spring-boot-quickstart.adoc index 83697f5fa73..f04c133f0e1 100644 --- a/docs/src/modules/ROOT/pages/quickstart/spring-boot/spring-boot-quickstart.adoc +++ b/docs/src/modules/ROOT/pages/quickstart/spring-boot/spring-boot-quickstart.adoc @@ -246,10 +246,10 @@ On the server side, the `info` log shows what Timefold Solver did in those five [source,options="nowrap"] ---- -... Solving started: time spent (33), best score (-8init/0hard/0soft), environment mode (REPRODUCIBLE), random (JDK with seed 0). +... Solving started: time spent (33), best score (-8init/0hard/0soft), environment mode (PHASE_ASSERT), random (JDK with seed 0). ... Construction Heuristic phase (0) ended: time spent (73), best score (0hard/0soft), move evaluation speed (459/sec), step total (4). ... Local Search phase (1) ended: time spent (5000), best score (0hard/0soft), move evaluation speed (28949/sec), step total (28398). -... Solving ended: time spent (5000), best score (0hard/0soft), move evaluation speed (28524/sec), phase total (2), environment mode (REPRODUCIBLE). +... Solving ended: time spent (5000), best score (0hard/0soft), move evaluation speed (28524/sec), phase total (2), environment mode (PHASE_ASSERT). ---- [NOTE] @@ -513,7 +513,7 @@ Use `debug` logging to show every _step_: [source,options="nowrap"] ---- -... Solving started: time spent (67), best score (-20init/0hard/0soft), environment mode (REPRODUCIBLE), random (JDK with seed 0). +... Solving started: time spent (67), best score (-20init/0hard/0soft), environment mode (PHASE_ASSERT), random (JDK with seed 0). ... CH step (0), time spent (128), score (-18init/0hard/0soft), selected move count (15), picked move ([Math(101) {null -> Room A}, Math(101) {null -> MONDAY 08:30}]). ... CH step (1), time spent (145), score (-16init/0hard/0soft), selected move count (15), picked move ([Physics(102) {null -> Room A}, Physics(102) {null -> MONDAY 09:30}]). ... diff --git a/docs/src/modules/ROOT/pages/using-timefold-solver/benchmarking-and-tweaking.adoc b/docs/src/modules/ROOT/pages/using-timefold-solver/benchmarking-and-tweaking.adoc index 926f6ea8f8d..d30a4b43556 100644 --- a/docs/src/modules/ROOT/pages/using-timefold-solver/benchmarking-and-tweaking.adoc +++ b/docs/src/modules/ROOT/pages/using-timefold-solver/benchmarking-and-tweaking.adoc @@ -1123,7 +1123,7 @@ The `subSingleCount` defaults to `1` (so no statistical benchmarking). ==== If `subSingleCount` is higher than ``1``, the benchmarker will automatically use a _different_ xref:using-timefold-solver/running-the-solver.adoc#randomNumberGenerator[`Random` seed] for every sub single run, -without losing reproducibility (for each sub single index) in xref:using-timefold-solver/running-the-solver.adoc#environmentMode[EnvironmentMode] ``REPRODUCIBLE`` and lower. +without losing reproducibility (for each sub single index) in xref:using-timefold-solver/running-the-solver.adoc#environmentMode[EnvironmentMode] ``NO_ASSERT`` and lower. ==== diff --git a/docs/src/modules/ROOT/pages/using-timefold-solver/running-the-solver.adoc b/docs/src/modules/ROOT/pages/using-timefold-solver/running-the-solver.adoc index 7d1d9f4048c..540594c25e5 100644 --- a/docs/src/modules/ROOT/pages/using-timefold-solver/running-the-solver.adoc +++ b/docs/src/modules/ROOT/pages/using-timefold-solver/running-the-solver.adoc @@ -130,7 +130,7 @@ You can set the environment mode in the solver configuration XML file: ---- - FAST_ASSERT + STEP_ASSERT ... ---- @@ -200,24 +200,24 @@ The `NON_INTRUSIVE_FULL_ASSERT` mode is horribly slow, because it does not rely on incremental score calculation. -[#environmentModeFastAssert] -=== `FAST_ASSERT` +[#environmentModeStepAssert] +=== `STEP_ASSERT` -The `FAST_ASSERT` mode turns on most assertions (such as assert that an undoMove's score is the same as before the Move) +The `STEP_ASSERT` mode turns on most assertions (such as assert that an undoMove's score is the same as before the Move) to fail-fast on a bug in a Move implementation, a constraint, the engine itself, ... This mode is <>. It is also intrusive because it calls the method `calculateScore()` more frequently than a non-assert mode. -The `FAST_ASSERT` mode is slow. +The `STEP_ASSERT` mode is slow. -It is recommended to write a test case that does a short run of your planning problem with the `FAST_ASSERT` mode on. +It is recommended to write a test case that does a short run of your planning problem with the `STEP_ASSERT` mode on. -[#environmentModeReproducible] -=== `REPRODUCIBLE` (default) +[#environmentModePhaseAssert] +=== `PHASE_ASSERT` (default) -The reproducible mode is the default mode because it is recommended during development. +The `PHASE_ASSERT` is the default mode because it is recommended during development. In this mode, two runs in the same Timefold Solver version will execute the same code in the same order. **Those two runs will have the same result at every step**, except if the note below applies. This enables you to reproduce bugs consistently. @@ -233,15 +233,18 @@ especially in the solution implementation. Replace it with ``LinkedHashSet``. * Combining a time gradient dependent algorithms (most notably Simulated Annealing) together with time spent termination. A sufficiently large difference in allocated CPU time will influence the time gradient values. Replace Simulated Annealing with Late Acceptance, or replace time spent termination with step count termination. - ==== -The reproducible mode can be slightly slower than the non-reproducible mode. +The `PHASE_ASSERT` mode can be slightly slower than the `NON_REPRODUCIBLE` mode. If your production environment can benefit from reproducibility, use this mode in production. In practice, this mode uses the default, fixed <> if no seed is specified, and it also disables certain concurrency optimizations, such as work stealing. +The `PHASE_ASSERT` mode is negligibly slower than the `NO_ASSERT` mode, +but it gives you the benefit of quickly checking for score corruptions. +If you can guarantee that your code is and will remain bug-free, +you can switch to the `NO_ASSERT` mode for a marginal performance gain. [#environmentModeProduction] === `NON_REPRODUCIBLE` @@ -292,7 +295,7 @@ For example, set it to `debug` logging, to see when the phases end and how fast [source,options="nowrap"] ---- -INFO Solving started: time spent (31), best score (-8init/0hard/0soft), environment mode (REPRODUCIBLE), move thread count (NONE), random (JDK with seed 0). +INFO Solving started: time spent (31), best score (-8init/0hard/0soft), environment mode (PHASE_ASSERT), move thread count (NONE), random (JDK with seed 0). INFO Problem scale: entity count (4), variable count (8), approximate value count (4), approximate problem scale (256). DEBUG CH step (0), time spent (47), score (-6init/0hard/0soft), selected move count (4), picked move ([Math(0) {null -> Room A}, Math(0) {null -> MONDAY 08:30}]). DEBUG CH step (1), time spent (50), score (-4init/0hard/0soft), selected move count (4), picked move ([Physics(1) {null -> Room A}, Physics(1) {null -> MONDAY 09:30}]). @@ -304,7 +307,7 @@ DEBUG LS step (1), time spent (60), score (-2hard/1soft), new best score (-2 DEBUG LS step (2), time spent (60), score (-2hard/0soft), best score (-2hard/1soft), accepted/selected move count (1/1), picked move (Math(0) {Room B, MONDAY 08:30} <-> Physics(1) {Room A, MONDAY 08:30}). ... INFO Local Search phase (1) ended: time spent (100), best score (0hard/1soft), move evaluation speed (2021/sec), step total (59). -INFO Solving ended: time spent (100), best score (0hard/1soft), move evaluation speed (1100/sec), phase total (2), environment mode (REPRODUCIBLE), move thread count (NONE). +INFO Solving ended: time spent (100), best score (0hard/1soft), move evaluation speed (1100/sec), phase total (2), environment mode (PHASE_ASSERT), move thread count (NONE). ---- All time spent values are in milliseconds. diff --git a/python/python-core/src/main/python/config/_config.py b/python/python-core/src/main/python/config/_config.py index fa714cfb92f..d48144df452 100644 --- a/python/python-core/src/main/python/config/_config.py +++ b/python/python-core/src/main/python/config/_config.py @@ -78,7 +78,7 @@ class EnvironmentMode(Enum): NON_REPRODUCIBLE = 'NON_REPRODUCIBLE' """ - The non reproducible mode is equally fast or slightly faster than REPRODUCIBLE. + The non reproducible mode is equally fast or slightly faster than NO_ASSERT. The random seed is different on every run, which makes it more robust against an unlucky random seed. An unlucky random seed gives a bad result on a certain data set with a certain solver configuration. @@ -87,9 +87,15 @@ class EnvironmentMode(Enum): In multithreaded scenarios, this mode allows the use of work stealing and other non deterministic speed tricks. """ - REPRODUCIBLE = 'REPRODUCIBLE' + NO_ASSERT = 'NO_ASSERT' """ - The reproducible mode is the default mode because it is recommended during development. + As defined by REPRODUCIBLE, + but negligibly faster on account of disabling all error detection. + """ + + PHASE_ASSERT = 'PHASE_ASSERT' + """ + The is the default mode because it is recommended during development. In this mode, 2 runs on the same computer will execute the same code in the same order. They will also yield the same result, except if they use a time based termination and they have a sufficiently large difference in allocated CPU time. @@ -102,11 +108,11 @@ class EnvironmentMode(Enum): and it also disables certain concurrency optimizations (such as work stealing). """ - FAST_ASSERT = 'FAST_ASSERT' + STEP_ASSERT = 'STEP_ASSERT' """ This mode turns on several assertions (but not all of them) to fail-fast on a bug in a Move implementation, a constraint rule, the engine itself or something else at a reasonable performance cost (in development at least). - This mode is reproducible (see REPRODUCIBLE mode). + This mode is reproducible (see PHASE_ASSERT mode). This mode is intrusive because it calls calculate_score more frequently than a non assert mode. This mode is slow. """ @@ -115,8 +121,8 @@ class EnvironmentMode(Enum): """ This mode turns on several assertions (but not all of them) to fail-fast on a bug in a Move implementation, a constraint, the engine itself or something else at an overwhelming performance cost. - This mode is reproducible (see REPRODUCIBLE mode). - This mode is non-intrusive, unlike FULL_ASSERT and FAST_ASSERT. + This mode is reproducible (see PHASE_ASSERT mode). + This mode is non-intrusive, unlike FULL_ASSERT and STEP_ASSERT. This mode is horribly slow. """ @@ -124,7 +130,7 @@ class EnvironmentMode(Enum): """ This mode turns on all assertions to fail-fast on a bug in a Move implementation, a constraint, the engine itself or something else at a horrible performance cost. - This mode is reproducible (see REPRODUCIBLE mode). + This mode is reproducible (see PHASE_ASSERT mode). This mode is intrusive because it calls calculate_score more frequently than a non assert mode. This mode is horribly slow. """ @@ -135,7 +141,7 @@ class EnvironmentMode(Enum): a constraint, the engine itself or something else at the highest performance cost. Because it tracks genuine and shadow variables, it is able to report precisely what variables caused the corruption and report any missed VariableListener events. - This mode is reproducible (see REPRODUCIBLE mode). + This mode is reproducible (see PHASE_ASSERT mode). This mode is intrusive because it calls calculate_score more frequently than a non assert mode. This mode is by far the slowest of all the modes. """ @@ -184,7 +190,7 @@ class SolverConfig(Generic[Solution_]): """ solution_class: Optional[type[Solution_]] = field(default=None) entity_class_list: Optional[list[type]] = field(default=None) - environment_mode: Optional[EnvironmentMode] = field(default=EnvironmentMode.REPRODUCIBLE) + environment_mode: Optional[EnvironmentMode] = field(default=EnvironmentMode.PHASE_ASSERT) random_seed: Optional[int] = field(default=None) move_thread_count: int | MoveThreadCount = field(default=MoveThreadCount.NONE) nearby_distance_meter_function: Optional[Callable[[Any, Any], float]] = field(default=None) diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversEmptyAppTest.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversEmptyAppTest.java index 4b15ff0ebf3..98682c05edf 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversEmptyAppTest.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversEmptyAppTest.java @@ -12,7 +12,7 @@ class TimefoldProcessorMultipleSolversEmptyAppTest { @RegisterExtension static final QuarkusUnitTest config = new QuarkusUnitTest() .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) .addClasses()); diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidConstraintClassTest.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidConstraintClassTest.java index 7e904025e13..cf33a3ed953 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidConstraintClassTest.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidConstraintClassTest.java @@ -30,7 +30,7 @@ class TimefoldProcessorMultipleSolversInvalidConstraintClassTest { @RegisterExtension static final QuarkusUnitTest config = new QuarkusUnitTest() .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) .addClasses(TestdataQuarkusEntity.class, TestdataQuarkusSolution.class)) .assertException(t -> assertThat(t) @@ -42,7 +42,7 @@ class TimefoldProcessorMultipleSolversInvalidConstraintClassTest { @RegisterExtension static final QuarkusUnitTest config2 = new QuarkusUnitTest() .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) .addClasses(TestdataQuarkusEntity.class, TestdataQuarkusSolution.class, DummyTestdataQuarkusEasyScoreCalculator.class) @@ -64,7 +64,7 @@ class TimefoldProcessorMultipleSolversInvalidConstraintClassTest { .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") .overrideConfigKey("quarkus.timefold.solver.\"solver1\".solver-config-xml", "ai/timefold/solver/quarkus/customSolverConfig.xml") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .overrideConfigKey("quarkus.timefold.solver.\"solver2\".solver-config-xml", "ai/timefold/solver/quarkus/customSolverConfig.xml") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) @@ -88,7 +88,7 @@ class TimefoldProcessorMultipleSolversInvalidConstraintClassTest { @RegisterExtension static final QuarkusUnitTest config4 = new QuarkusUnitTest() .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) .addClasses(TestdataQuarkusEntity.class, TestdataQuarkusSolution.class, TestdataQuarkusConstraintProvider.class) @@ -111,7 +111,7 @@ class TimefoldProcessorMultipleSolversInvalidConstraintClassTest { .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") .overrideConfigKey("quarkus.timefold.solver.\"solver1\".solver-config-xml", "ai/timefold/solver/quarkus/customSolverConfigWithoutScore.xml") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .overrideConfigKey("quarkus.timefold.solver.\"solver2\".solver-config-xml", "ai/timefold/solver/quarkus/customSolverConfigWithoutScore.xml") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) @@ -134,7 +134,7 @@ class TimefoldProcessorMultipleSolversInvalidConstraintClassTest { @RegisterExtension static final QuarkusUnitTest config6 = new QuarkusUnitTest() .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) .addClasses(TestdataQuarkusEntity.class, TestdataQuarkusSolution.class, DummyTestdataQuarkusIncrementalScoreCalculator.class) @@ -157,7 +157,7 @@ class TimefoldProcessorMultipleSolversInvalidConstraintClassTest { .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") .overrideConfigKey("quarkus.timefold.solver.\"solver1\".solver-config-xml", "ai/timefold/solver/quarkus/customSolverConfigWithoutScore.xml") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .overrideConfigKey("quarkus.timefold.solver.\"solver2\".solver-config-xml", "ai/timefold/solver/quarkus/customSolverConfigWithoutScore.xml") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidEntityClassTest.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidEntityClassTest.java index f1f69b06ad8..b370627f0a6 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidEntityClassTest.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidEntityClassTest.java @@ -23,7 +23,7 @@ class TimefoldProcessorMultipleSolversInvalidEntityClassTest { @RegisterExtension static final QuarkusUnitTest config1 = new QuarkusUnitTest() .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class).addClasses(TestdataQuarkusSolution.class)) .assertException(t -> assertThat(t) .isInstanceOf(IllegalStateException.class) diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidSolutionClassTest.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidSolutionClassTest.java index 171a550299c..e74ea9073c2 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidSolutionClassTest.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversInvalidSolutionClassTest.java @@ -30,7 +30,7 @@ class TimefoldProcessorMultipleSolversInvalidSolutionClassTest { @RegisterExtension static final QuarkusUnitTest config1 = new QuarkusUnitTest() .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class).addClasses(TestdataQuarkusEntity.class)) .assertException(t -> assertThat(t) .isInstanceOf(IllegalStateException.class) @@ -41,7 +41,7 @@ class TimefoldProcessorMultipleSolversInvalidSolutionClassTest { @RegisterExtension static final QuarkusUnitTest config2 = new QuarkusUnitTest() .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) .addClasses(TestdataQuarkusEntity.class, TestdataQuarkusSolution.class, TestdataQuarkusConstraintProvider.class) diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversXMLPropertyTest.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversXMLPropertyTest.java index 23ae42dccac..bb697d4c0e6 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversXMLPropertyTest.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorMultipleSolversXMLPropertyTest.java @@ -26,7 +26,7 @@ class TimefoldProcessorMultipleSolversXMLPropertyTest { .overrideConfigKey("quarkus.timefold.solver.\"solver1\".environment-mode", "FULL_ASSERT") .overrideConfigKey("quarkus.timefold.solver.\"solver1\".solver-config-xml", "ai/timefold/solver/quarkus/customSolverQuarkusConfig.xml") - .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "REPRODUCIBLE") + .overrideConfigKey("quarkus.timefold.solver.\"solver2\".environment-mode", "PHASE_ASSERT") .overrideConfigKey("quarkus.timefold.solver.\"solver2\".solver-config-xml", "ai/timefold/solver/quarkus/customSolverQuarkusShadowVariableConfig.xml") .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) diff --git a/quarkus-integration/quarkus/runtime/src/main/java/ai/timefold/solver/quarkus/config/SolverRuntimeConfig.java b/quarkus-integration/quarkus/runtime/src/main/java/ai/timefold/solver/quarkus/config/SolverRuntimeConfig.java index 82b9889c98a..671520a238e 100644 --- a/quarkus-integration/quarkus/runtime/src/main/java/ai/timefold/solver/quarkus/config/SolverRuntimeConfig.java +++ b/quarkus-integration/quarkus/runtime/src/main/java/ai/timefold/solver/quarkus/config/SolverRuntimeConfig.java @@ -16,7 +16,7 @@ public interface SolverRuntimeConfig { /** * Enable runtime assertions to detect common bugs in your implementation during development. - * Defaults to {@link EnvironmentMode#REPRODUCIBLE}. + * Defaults to {@link EnvironmentMode#PHASE_ASSERT}. */ Optional environmentMode(); diff --git a/spring-integration/spring-boot-autoconfigure/src/main/java/ai/timefold/solver/spring/boot/autoconfigure/config/SolverProperties.java b/spring-integration/spring-boot-autoconfigure/src/main/java/ai/timefold/solver/spring/boot/autoconfigure/config/SolverProperties.java index 186d3b7f987..175ce129a18 100644 --- a/spring-integration/spring-boot-autoconfigure/src/main/java/ai/timefold/solver/spring/boot/autoconfigure/config/SolverProperties.java +++ b/spring-integration/spring-boot-autoconfigure/src/main/java/ai/timefold/solver/spring/boot/autoconfigure/config/SolverProperties.java @@ -19,7 +19,7 @@ public class SolverProperties { /** * Enable runtime assertions to detect common bugs in your implementation during development. - * Defaults to "REPRODUCIBLE". + * Defaults to {@link EnvironmentMode#PHASE_ASSERT}. */ private EnvironmentMode environmentMode; From 9a80f10b474a98bbfb449c9c32a0e9fd89c33e65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 07:49:29 +0100 Subject: [PATCH 03/14] Update core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Frederico Gonçalves --- .../solver/core/impl/score/director/AbstractScoreDirector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java index 69d95ff0da8..57a1634649b 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java @@ -583,7 +583,7 @@ public void assertShadowVariablesAreNotStale(Score_ expectedWorkingScore, Object "assertShadowVariablesAreNotStale(" + expectedWorkingScore + ", " + completedAction + ")"); throw new VariableCorruptionException(""" Impossible %s corruption (%s): the expectedWorkingScore (%s) is not the workingScore (%s) \ - after all %ss were triggered without changes to the genuine variables after completedAction (%s). + after all %s were triggered without changes to the genuine variables after completedAction (%s). All the shadow variable values are still the same, so this is impossible. Maybe run with %s if you haven't already, to fail earlier.""" .formatted(VariableListener.class.getSimpleName(), From e3301e1580d666feb988001b805b667568dd4c04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 08:43:39 +0100 Subject: [PATCH 04/14] Minor improvements --- .../core/config/solver/EnvironmentMode.java | 55 ++++++++++--------- .../decider/ConstructionHeuristicDecider.java | 4 +- .../DefaultExhaustiveSearchPhase.java | 4 +- .../DefaultExhaustiveSearchPhaseFactory.java | 7 +-- .../decider/LocalSearchDecider.java | 4 +- .../decider/acceptor/AcceptorFactory.java | 6 +- .../solver/core/impl/phase/AbstractPhase.java | 6 +- .../director/ScoreDirectorFactoryFactory.java | 2 +- .../recaller/BestSolutionRecallerFactory.java | 2 +- .../config/solver/EnvironmentModeTest.java | 52 ++++++++++-------- .../running-the-solver.adoc | 2 +- 11 files changed, 76 insertions(+), 68 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java b/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java index e967d84bb16..b16e1bcd0e5 100644 --- a/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java +++ b/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java @@ -131,47 +131,48 @@ public enum EnvironmentMode { } public boolean isStepAssertOrMore() { - return switch (this) { - case NON_REPRODUCIBLE, NO_ASSERT, PHASE_ASSERT, STEP_ASSERT, NON_INTRUSIVE_FULL_ASSERT, FULL_ASSERT, - TRACKED_FULL_ASSERT -> - isAsserted() && this != PHASE_ASSERT; - case REPRODUCIBLE -> PHASE_ASSERT.isAsserted(); - case FAST_ASSERT -> STEP_ASSERT.isAsserted(); - }; + if (!isAsserted()) { + return false; + } + return this != PHASE_ASSERT; } public boolean isAsserted() { return asserted; } - public boolean isNonIntrusiveFullAsserted() { + public boolean isFullyAsserted() { return switch (this) { - case NON_REPRODUCIBLE, NO_ASSERT, PHASE_ASSERT, STEP_ASSERT, NON_INTRUSIVE_FULL_ASSERT, FULL_ASSERT, - TRACKED_FULL_ASSERT -> { - if (!isStepAssertOrMore()) { - yield false; - } - yield this != STEP_ASSERT; - } - case REPRODUCIBLE -> PHASE_ASSERT.isNonIntrusiveFullAsserted(); - case FAST_ASSERT -> STEP_ASSERT.isNonIntrusiveFullAsserted(); + case TRACKED_FULL_ASSERT, FULL_ASSERT, NON_INTRUSIVE_FULL_ASSERT -> true; + case STEP_ASSERT, FAST_ASSERT, PHASE_ASSERT, REPRODUCIBLE, NO_ASSERT, NON_REPRODUCIBLE -> false; }; } - public boolean isIntrusiveStepAsserted() { + /** + * @deprecated Use {@link #isFullyAsserted()} instead. + */ + @Deprecated(forRemoval = true, since = "1.20.0") + public boolean isNonIntrusiveFullAsserted() { + return isFullyAsserted(); + } + + public boolean isIntrusivelyAsserted() { return switch (this) { - case NON_REPRODUCIBLE, NO_ASSERT, PHASE_ASSERT, STEP_ASSERT, NON_INTRUSIVE_FULL_ASSERT, FULL_ASSERT, - TRACKED_FULL_ASSERT -> { - if (!isStepAssertOrMore()) { - yield false; - } - yield this != NON_INTRUSIVE_FULL_ASSERT; - } - case REPRODUCIBLE -> PHASE_ASSERT.isIntrusiveStepAsserted(); - case FAST_ASSERT -> STEP_ASSERT.isIntrusiveStepAsserted(); + // STEP_ASSERT = former FAST_ASSERT + case TRACKED_FULL_ASSERT, FULL_ASSERT, STEP_ASSERT, FAST_ASSERT -> true; + // NO_ASSERT = former REPRODUCIBLE + case NON_INTRUSIVE_FULL_ASSERT, PHASE_ASSERT, NO_ASSERT, NON_REPRODUCIBLE, REPRODUCIBLE -> false; }; } + /** + * @deprecated Use {@link #isIntrusivelyAsserted()} instead. + */ + @Deprecated(forRemoval = true, since = "1.20.0") + public boolean isIntrusiveFastAsserted() { + return isIntrusivelyAsserted(); + } + public boolean isReproducible() { return this != NON_REPRODUCIBLE; } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/decider/ConstructionHeuristicDecider.java b/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/decider/ConstructionHeuristicDecider.java index e266db05443..be5e624193f 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/decider/ConstructionHeuristicDecider.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/decider/ConstructionHeuristicDecider.java @@ -50,8 +50,8 @@ public ConstructionHeuristicForager getForager() { } public void enableAssertions(EnvironmentMode environmentMode) { - this.assertMoveScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); - this.assertExpectedUndoMoveScore = environmentMode.isIntrusiveStepAsserted(); + this.assertMoveScoreFromScratch = environmentMode.isFullyAsserted(); + this.assertExpectedUndoMoveScore = environmentMode.isIntrusivelyAsserted(); } // ************************************************************************ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java index c1058dfa036..a14a01405be 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java @@ -248,8 +248,8 @@ public Builder(int phaseIndex, String logIndentation, Termination pha @Override public Builder enableAssertions(EnvironmentMode environmentMode) { super.enableAssertions(environmentMode); - assertWorkingSolutionScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); - assertExpectedWorkingSolutionScore = environmentMode.isIntrusiveStepAsserted(); + assertWorkingSolutionScoreFromScratch = environmentMode.isFullyAsserted(); + assertExpectedWorkingSolutionScore = environmentMode.isIntrusivelyAsserted(); return this; } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java index 769a7378ffb..c81d8f4ea15 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseFactory.java @@ -140,13 +140,12 @@ private ExhaustiveSearchDecider buildDecider(HeuristicConfigPolicy decider = new ExhaustiveSearchDecider<>(configPolicy.getLogIndentation(), - bestSolutionRecaller, termination, - manualEntityMimicRecorder, moveSelector, scoreBounderEnabled, scoreBounder); + bestSolutionRecaller, termination, manualEntityMimicRecorder, moveSelector, scoreBounderEnabled, scoreBounder); EnvironmentMode environmentMode = configPolicy.getEnvironmentMode(); - if (environmentMode.isNonIntrusiveFullAsserted()) { + if (environmentMode.isFullyAsserted()) { decider.setAssertMoveScoreFromScratch(true); } - if (environmentMode.isIntrusiveStepAsserted()) { + if (environmentMode.isIntrusivelyAsserted()) { decider.setAssertExpectedUndoMoveScore(true); } return decider; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/LocalSearchDecider.java b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/LocalSearchDecider.java index 437bd85b4ff..f84d778e346 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/LocalSearchDecider.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/LocalSearchDecider.java @@ -62,8 +62,8 @@ public LocalSearchForager getForager() { } public void enableAssertions(EnvironmentMode environmentMode) { - assertMoveScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); - assertExpectedUndoMoveScore = environmentMode.isIntrusiveStepAsserted(); + assertMoveScoreFromScratch = environmentMode.isFullyAsserted(); + assertExpectedUndoMoveScore = environmentMode.isIntrusivelyAsserted(); } // ************************************************************************ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/AcceptorFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/AcceptorFactory.java index f1d2232668d..a73b33e68c2 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/AcceptorFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/AcceptorFactory.java @@ -123,7 +123,7 @@ private Optional> buildEntityTabuAcceptor(Heuristi acceptor.setFadingTabuSizeStrategy( new EntityRatioTabuSizeStrategy<>(acceptorConfig.getFadingEntityTabuRatio())); } - if (configPolicy.getEnvironmentMode().isNonIntrusiveFullAsserted()) { + if (configPolicy.getEnvironmentMode().isFullyAsserted()) { acceptor.setAssertTabuHashCodeCorrectness(true); } return Optional.of(acceptor); @@ -171,7 +171,7 @@ private Optional> buildValueTabuAcceptor(HeuristicC if (acceptorConfig.getFadingValueTabuSize() != null) { acceptor.setFadingTabuSizeStrategy(new FixedTabuSizeStrategy<>(acceptorConfig.getFadingValueTabuSize())); } - if (configPolicy.getEnvironmentMode().isNonIntrusiveFullAsserted()) { + if (configPolicy.getEnvironmentMode().isFullyAsserted()) { acceptor.setAssertTabuHashCodeCorrectness(true); } return Optional.of(acceptor); @@ -189,7 +189,7 @@ private Optional> buildMoveTabuAcceptor(HeuristicCon if (acceptorConfig.getFadingMoveTabuSize() != null) { acceptor.setFadingTabuSizeStrategy(new FixedTabuSizeStrategy<>(acceptorConfig.getFadingMoveTabuSize())); } - if (configPolicy.getEnvironmentMode().isNonIntrusiveFullAsserted()) { + if (configPolicy.getEnvironmentMode().isFullyAsserted()) { acceptor.setAssertTabuHashCodeCorrectness(true); } return Optional.of(acceptor); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java b/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java index 8a4e94c4054..6103c90bfb8 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java @@ -270,9 +270,9 @@ protected AbstractPhaseBuilder(int phaseIndex, String logIndentation, Terminatio public AbstractPhaseBuilder enableAssertions(EnvironmentMode environmentMode) { assertPhaseScoreFromScratch = environmentMode.isAsserted(); - assertStepScoreFromScratch = environmentMode.isNonIntrusiveFullAsserted(); - assertExpectedStepScore = environmentMode.isIntrusiveStepAsserted(); - assertShadowVariablesAreNotStaleAfterStep = environmentMode.isIntrusiveStepAsserted(); + assertStepScoreFromScratch = environmentMode.isFullyAsserted(); + assertExpectedStepScore = environmentMode.isIntrusivelyAsserted(); + assertShadowVariablesAreNotStaleAfterStep = environmentMode.isIntrusivelyAsserted(); return this; } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactory.java index a2158f8fcb2..73edf9676b9 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactoryFactory.java @@ -46,7 +46,7 @@ public InnerScoreDirectorFactory buildScoreDirectorFactory(En config.getInitializingScoreTrend() == null ? InitializingScoreTrendLevel.ANY.name() : config.getInitializingScoreTrend(), solutionDescriptor.getScoreDefinition().getLevelsSize())); - if (environmentMode.isNonIntrusiveFullAsserted()) { + if (environmentMode.isFullyAsserted()) { scoreDirectorFactory.setAssertClonedSolution(true); } if (environmentMode.isTracking()) { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/recaller/BestSolutionRecallerFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/recaller/BestSolutionRecallerFactory.java index d04e16bf712..c56f244290b 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/recaller/BestSolutionRecallerFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/recaller/BestSolutionRecallerFactory.java @@ -10,7 +10,7 @@ public static BestSolutionRecallerFactory create() { public BestSolutionRecaller buildBestSolutionRecaller(EnvironmentMode environmentMode) { BestSolutionRecaller bestSolutionRecaller = new BestSolutionRecaller<>(); - if (environmentMode.isNonIntrusiveFullAsserted()) { + if (environmentMode.isFullyAsserted()) { bestSolutionRecaller.setAssertInitialScoreFromScratch(true); bestSolutionRecaller.setAssertShadowVariablesAreNotStale(true); bestSolutionRecaller.setAssertBestScoreIsUnmodified(true); diff --git a/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java b/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java index 3a7816f01f1..f664892a449 100644 --- a/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java +++ b/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java @@ -1,6 +1,7 @@ package ai.timefold.solver.core.config.solver; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.SoftAssertions.assertSoftly; import java.util.ArrayList; @@ -38,14 +39,17 @@ import ai.timefold.solver.core.impl.testdata.util.PlannerTestUtils; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; +@Execution(ExecutionMode.CONCURRENT) class EnvironmentModeTest { private static final int NUMBER_OF_RANDOM_NUMBERS_GENERATED = 1000; private static final int NUMBER_OF_TIMES_RUN = 10; - private static final int NUMBER_OF_TERMINATION_STEP_COUNT_LIMIT = 20; + private static final long NUMBER_OF_TERMINATION_MOVE_COUNT_LIMIT = 1_000L; private static TestdataSolution inputProblem; @@ -65,7 +69,7 @@ private static SolverConfig buildSolverConfig(EnvironmentMode environmentMode) { LocalSearchPhaseConfig localSearchPhaseConfig = new LocalSearchPhaseConfig(); localSearchPhaseConfig .setTerminationConfig( - new TerminationConfig().withStepCountLimit(NUMBER_OF_TERMINATION_STEP_COUNT_LIMIT)); + new TerminationConfig().withMoveCountLimit(NUMBER_OF_TERMINATION_MOVE_COUNT_LIMIT)); return new SolverConfig() .withSolutionClass(TestdataSolution.class) @@ -83,18 +87,10 @@ void determinism(EnvironmentMode environmentMode) { Solver solver1 = SolverFactory. create(solverConfig).buildSolver(); Solver solver2 = SolverFactory. create(solverConfig).buildSolver(); - switch (environmentMode) { - case NON_REPRODUCIBLE -> assertNonReproducibility(solver1, solver2); - case TRACKED_FULL_ASSERT, - FULL_ASSERT, - STEP_ASSERT, - NON_INTRUSIVE_FULL_ASSERT, - NO_ASSERT, - PHASE_ASSERT, - FAST_ASSERT, - REPRODUCIBLE -> - assertReproducibility(solver1, solver2); - default -> throw new IllegalStateException("Unexpected value: " + environmentMode); + if (environmentMode.isReproducible()) { + assertReproducibility(solver1, solver2); + } else { + assertNonReproducibility(solver1, solver2); } } @@ -154,9 +150,22 @@ void corruptedUndoShadowVariableListener(EnvironmentMode environmentMode) { .withMessageContainingAll("corrupted undoMove"); } case PHASE_ASSERT, NO_ASSERT, NON_REPRODUCIBLE, NON_INTRUSIVE_FULL_ASSERT, REPRODUCIBLE -> { - // No exception expected + var e1 = new CorruptedUndoShadowEntity("e1"); + var e2 = new CorruptedUndoShadowEntity("e2"); + var v1 = new CorruptedUndoShadowValue("v1"); + var v2 = new CorruptedUndoShadowValue("v2"); + + e1.setValue(v1); + e1.setValueClone(v1); + v1.setEntities(new ArrayList<>(List.of(e1))); + + e2.setValue(v2); + e2.setValueClone(v2); + v2.setEntities(new ArrayList<>(List.of(e2))); + assertThatNoException() + .isThrownBy(() -> PlannerTestUtils.solve(solverConfig, + new CorruptedUndoShadowSolution(List.of(e1, e2), List.of(v1, v2)), false)); } - default -> throw new IllegalStateException("Unexpected value: " + environmentMode); } } @@ -168,15 +177,14 @@ void corruptedConstraints(EnvironmentMode environmentMode) { setSolverConfigCalculatorClass(solverConfig, TestdataCorruptedDifferentValuesCalculator.class); switch (environmentMode) { - case TRACKED_FULL_ASSERT -> - assertIllegalStateExceptionWhileSolving(solverConfig, "not the uncorruptedScore"); - case FULL_ASSERT, NON_INTRUSIVE_FULL_ASSERT -> + case TRACKED_FULL_ASSERT, FULL_ASSERT, NON_INTRUSIVE_FULL_ASSERT -> assertIllegalStateExceptionWhileSolving(solverConfig, "not the uncorruptedScore"); case STEP_ASSERT -> assertIllegalStateExceptionWhileSolving(solverConfig, "Score corruption analysis could not be generated "); - case PHASE_ASSERT, NO_ASSERT, NON_REPRODUCIBLE -> { - // No exception expected - } + case PHASE_ASSERT -> + assertIllegalStateExceptionWhileSolving(solverConfig, "Solver corruption was detected."); + case NO_ASSERT, NON_REPRODUCIBLE -> assertThatNoException() + .isThrownBy(() -> PlannerTestUtils.solve(solverConfig, inputProblem)); } } diff --git a/docs/src/modules/ROOT/pages/using-timefold-solver/running-the-solver.adoc b/docs/src/modules/ROOT/pages/using-timefold-solver/running-the-solver.adoc index 540594c25e5..9e5bf2985de 100644 --- a/docs/src/modules/ROOT/pages/using-timefold-solver/running-the-solver.adoc +++ b/docs/src/modules/ROOT/pages/using-timefold-solver/running-the-solver.adoc @@ -194,7 +194,7 @@ The `NON_INTRUSIVE_FULL_ASSERT` turns on several assertions to fail-fast on a bu a constraint, the engine itself, ... This mode is <>. -It is non-intrusive because it does not call the method `calculateScore()` more frequently than a non assert mode. +It is non-intrusive because it does not call the method `calculateScore()` more frequently than a non-assert mode. The `NON_INTRUSIVE_FULL_ASSERT` mode is horribly slow, because it does not rely on incremental score calculation. From 26fa308ffc60e102ae56565d769ba79e78ca7f5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 08:57:14 +0100 Subject: [PATCH 05/14] Recipe --- .../core/config/solver/EnvironmentMode.java | 3 +- .../upgrade-to-latest-version.adoc | 37 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java b/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java index b16e1bcd0e5..4e97c35370d 100644 --- a/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java +++ b/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java @@ -7,6 +7,7 @@ import jakarta.xml.bind.annotation.XmlEnum; import ai.timefold.solver.core.api.domain.entity.PlanningEntity; +import ai.timefold.solver.core.api.domain.variable.VariableListener; import ai.timefold.solver.core.api.solver.Solver; import ai.timefold.solver.core.impl.heuristic.move.Move; import ai.timefold.solver.core.impl.score.director.InnerScoreDirector; @@ -28,7 +29,7 @@ public enum EnvironmentMode { * a constraint, the engine itself or something else at the highest performance cost. *

* Because it tracks genuine and shadow variables, it is able to report precisely what variables caused the corruption and - * report any missed {@link ai.timefold.solver.core.api.domain.variable.VariableListener} events. + * report any missed {@link VariableListener} events. *

* This mode is reproducible (see {@link #PHASE_ASSERT} mode). *

diff --git a/docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-to-latest-version.adoc b/docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-to-latest-version.adoc index c4131e3ccd6..c5a9b1da2fb 100644 --- a/docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-to-latest-version.adoc +++ b/docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-to-latest-version.adoc @@ -57,6 +57,43 @@ Every upgrade note indicates how likely your code will be affected by that chang The upgrade recipe often lists the changes as they apply to Java code. We kindly ask Kotlin and Python users to translate the changes accordingly. +=== Upgrade from 1.19.0 to 1.20.0 + +.icon:info-circle[role=yellow] New values for `EnvironmentMode` +[%collapsible%open] +==== +We've made the following changes to the `EnvironmentMode` enum: + +- `EnvironmentMode.REPRODUCIBLE` became `EnvironmentMode.NO_ASSERT`, +- `EnvironmentMode.FAST_ASSERT` became `EnvironmentMode.STEP_ASSERT`, +- and `EnvironmentMode.PHASE_ASSERT` was added. + +`EnvironmentMode.PHASE_ASSERT` is the new default mode, +and it performs minimal assertions at the end of each phase. +To restore the original entirely unasserted behavior, +use `EnvironmentMode.NO_ASSERT`, +but we strongly recommend using the new default mode instead. + +Before in `*.java`: + +[source,java] +---- +var solverConfig = new SolverConfig() + .withEnvironmentMode(EnvironmentMode.REPRODUCIBLE); + ... +---- + +After in `*.java`: + +[source,java] +---- +var solverConfig = new SolverConfig() + .withEnvironmentMode(EnvironmentMode.NO_ASSERT); + ... +---- +==== + + === Upgrade from 1.18.0 to 1.19.0 .icon:info-circle[role=yellow] New argument to `FirstInitializedSolutionConsumer` From 02a29d22137350456a7395d4b9f0d21fd41385e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 09:02:05 +0100 Subject: [PATCH 06/14] Finishing touches --- .../solver/benchmark/impl/report/BenchmarkReport.java | 6 +++--- .../solver/core/config/solver/EnvironmentMode.java | 10 +++++----- .../solver/core/impl/solver/DefaultSolverTest.java | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/benchmark/src/main/java/ai/timefold/solver/benchmark/impl/report/BenchmarkReport.java b/benchmark/src/main/java/ai/timefold/solver/benchmark/impl/report/BenchmarkReport.java index 6ec2c0615ef..d12d0ee41cd 100644 --- a/benchmark/src/main/java/ai/timefold/solver/benchmark/impl/report/BenchmarkReport.java +++ b/benchmark/src/main/java/ai/timefold/solver/benchmark/impl/report/BenchmarkReport.java @@ -318,9 +318,9 @@ public List getWarningList() { EnvironmentMode environmentMode = plannerBenchmarkResult.getEnvironmentMode(); if (environmentMode != null && environmentMode.isStepAssertOrMore()) { // Phase assert performance impact is negligible. - warningList.add("The environmentMode (" + environmentMode + ") is asserting." - + " This decreases performance." - + " Maybe set the environmentMode to " + EnvironmentMode.PHASE_ASSERT + "."); + warningList.add( + "The environmentMode (%s) is step-asserting or more. This decreases performance. Maybe set the environmentMode to %s." + .formatted(environmentMode, EnvironmentMode.PHASE_ASSERT)); } LoggingLevel loggingLevelTimefoldCore = plannerBenchmarkResult.getLoggingLevelTimefoldSolverCore(); if (loggingLevelTimefoldCore == LoggingLevel.TRACE) { diff --git a/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java b/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java index 4e97c35370d..10cb1c0d7db 100644 --- a/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java +++ b/core/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java @@ -64,6 +64,11 @@ public enum EnvironmentMode { * This mode is horribly slow. */ NON_INTRUSIVE_FULL_ASSERT(true), + /** + * @deprecated Prefer {@link #STEP_ASSERT}. + */ + @Deprecated(forRemoval = true, since = "1.20.0") + FAST_ASSERT(true), /** * This mode turns on several assertions to fail-fast * on a bug in a {@link Move} implementation, a constraint rule, the engine itself or something else @@ -77,11 +82,6 @@ public enum EnvironmentMode { * This mode is slow. */ STEP_ASSERT(true), - /** - * @deprecated Prefer {@link #STEP_ASSERT}. - */ - @Deprecated(forRemoval = true, since = "1.20.0") - FAST_ASSERT(true), /** * This is the default mode as it is recommended during development, * and runs minimal correctness checks that serve to quickly identify score corruption bugs. diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index 3521bdaef92..92c6a7cf0ef 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -128,7 +128,7 @@ void solve() { } @Test - void solveCorruptedEasyGuarded() { + void solveCorruptedEasyPhaseAsserted() { var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class) .withEnvironmentMode(EnvironmentMode.PHASE_ASSERT) .withEasyScoreCalculatorClass(CorruptedEasyScoreCalculator.class); @@ -144,7 +144,7 @@ void solveCorruptedEasyGuarded() { } @Test - void solveCorruptedEasyUnguarded() { + void solveCorruptedEasyUnasserted() { var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class) .withEnvironmentMode(EnvironmentMode.NO_ASSERT) .withEasyScoreCalculatorClass(CorruptedEasyScoreCalculator.class); From b819f8ad69e7c2d6b26bd097b8046ecbf4bc450a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 11:54:30 +0100 Subject: [PATCH 07/14] Fix XSDs --- benchmark/src/main/resources/benchmark.xsd | 4 ++-- core/src/main/resources/solver.xsd | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/benchmark/src/main/resources/benchmark.xsd b/benchmark/src/main/resources/benchmark.xsd index 16baee7d16c..0023b03f5d0 100644 --- a/benchmark/src/main/resources/benchmark.xsd +++ b/benchmark/src/main/resources/benchmark.xsd @@ -2627,10 +2627,10 @@ - + - + diff --git a/core/src/main/resources/solver.xsd b/core/src/main/resources/solver.xsd index cab8715c17f..592c85741c7 100644 --- a/core/src/main/resources/solver.xsd +++ b/core/src/main/resources/solver.xsd @@ -1565,10 +1565,10 @@ - - + + From 9b06660d2d5ea29bc21fc362ec88fd0a9b15c5d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 12:19:18 +0100 Subject: [PATCH 08/14] Attempt to fix the flakiness --- .../impl/statistic/BestScoreStatistic.java | 3 +- ...DefaultConstructionHeuristicPhaseTest.java | 4 +-- .../DefaultExhaustiveSearchPhaseTest.java | 4 +-- .../generic/RuinRecreateMoveSelectorTest.java | 4 +-- .../ListRuinRecreateMoveSelectorTest.java | 4 +-- .../DefaultLocalSearchPhaseTest.java | 4 +-- .../core/impl/solver/DefaultSolverTest.java | 31 ++++++++++--------- .../core/impl/testutil/TestMeterRegistry.java | 8 ++--- 8 files changed, 30 insertions(+), 32 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/statistic/BestScoreStatistic.java b/core/src/main/java/ai/timefold/solver/core/impl/statistic/BestScoreStatistic.java index b77e6621ced..181263bb6b7 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/statistic/BestScoreStatistic.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/statistic/BestScoreStatistic.java @@ -15,8 +15,8 @@ import io.micrometer.core.instrument.Tags; public class BestScoreStatistic implements SolverStatistic { - private final Map>> tagsToBestScoreMap = new ConcurrentHashMap<>(); + private final Map>> tagsToBestScoreMap = new ConcurrentHashMap<>(); private final Map, SolverEventListener> solverToEventListenerMap = new WeakHashMap<>(); @Override @@ -25,6 +25,7 @@ public void unregister(Solver solver) { if (listener != null) { solver.removeEventListener(listener); } + tagsToBestScoreMap.clear(); } @Override diff --git a/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java index aa9f7195bd6..91d7cd4f2f2 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java @@ -120,7 +120,7 @@ void solveWithCustomMetrics() { ((DefaultSolver) solver).addPhaseLifecycleListener(new PhaseLifecycleListenerAdapter<>() { @Override public void solvingEnded(SolverScope solverScope) { - meterRegistry.publish(solver); + meterRegistry.publish(); var changeMoveKey = "ChangeMove(TestdataEntity.value)"; if (solverScope.getMoveCountTypes().contains(changeMoveKey)) { var counter = meterRegistry @@ -156,7 +156,7 @@ void solveWithListVariableAndCustomMetrics() { ((DefaultSolver) solver).addPhaseLifecycleListener(new PhaseLifecycleListenerAdapter<>() { @Override public void solvingEnded(SolverScope solverScope) { - meterRegistry.publish(solver); + meterRegistry.publish(); var changeMoveKey = "ListAssignMove(TestdataListEntity.valueList)"; if (solverScope.getMoveCountTypes().contains(changeMoveKey)) { var counter = meterRegistry diff --git a/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java index fcdd9b4a202..3c696b659ec 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java @@ -180,7 +180,7 @@ void solveWithInitializedEntitiesAndMetric() { SolverMetric.MOVE_EVALUATION_COUNT.register(solver); SolverMetric.SCORE_CALCULATION_COUNT.register(solver); - meterRegistry.publish(solver); + meterRegistry.publish(); var scoreCount = meterRegistry.getMeasurement(SolverMetric.SCORE_CALCULATION_COUNT.getMeterId(), "VALUE"); var moveCount = meterRegistry.getMeasurement(SolverMetric.MOVE_EVALUATION_COUNT.getMeterId(), "VALUE"); assertThat(scoreCount).isPositive(); @@ -213,7 +213,7 @@ void solveCustomMetrics() { ((DefaultSolver) solver).addPhaseLifecycleListener(new PhaseLifecycleListenerAdapter<>() { @Override public void solvingEnded(SolverScope solverScope) { - meterRegistry.publish(solver); + meterRegistry.publish(); var changeMoveKey = "ChangeMove(TestdataEntity.value)"; if (solverScope.getMoveCountTypes().contains(changeMoveKey)) { var counter = meterRegistry diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java index 0d6cf3589a2..8a0efa51b5b 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java @@ -66,12 +66,12 @@ void testRuiningWithMetric() { .withMoveSelectorConfig(new RuinRecreateMoveSelectorConfig()))); var problem = TestdataSolution.generateSolution(5, 30); var solver = SolverFactory.create(solverConfig).buildSolver(); - solver.addEventListener(event -> meterRegistry.publish(solver)); + solver.addEventListener(event -> meterRegistry.publish()); solver.solve(problem); SolverMetric.MOVE_EVALUATION_COUNT.register(solver); SolverMetric.SCORE_CALCULATION_COUNT.register(solver); - meterRegistry.publish(solver); + meterRegistry.publish(); var scoreCount = meterRegistry.getMeasurement(SolverMetric.SCORE_CALCULATION_COUNT.getMeterId(), "VALUE"); var moveCount = meterRegistry.getMeasurement(SolverMetric.MOVE_EVALUATION_COUNT.getMeterId(), "VALUE"); assertThat(scoreCount).isPositive(); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java index 1b0e646d456..0213c69a1c0 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java @@ -92,12 +92,12 @@ void testRuiningWithMetric() { .withMoveSelectorConfig(new ListRuinRecreateMoveSelectorConfig()))); var problem = TestdataListSolution.generateUninitializedSolution(10, 3); var solver = SolverFactory.create(solverConfig).buildSolver(); - solver.addEventListener(event -> meterRegistry.publish(solver)); + solver.addEventListener(event -> meterRegistry.publish()); solver.solve(problem); SolverMetric.MOVE_EVALUATION_COUNT.register(solver); SolverMetric.SCORE_CALCULATION_COUNT.register(solver); - meterRegistry.publish(solver); + meterRegistry.publish(); var scoreCount = meterRegistry.getMeasurement(SolverMetric.SCORE_CALCULATION_COUNT.getMeterId(), "VALUE"); var moveCount = meterRegistry.getMeasurement(SolverMetric.MOVE_EVALUATION_COUNT.getMeterId(), "VALUE"); assertThat(scoreCount).isPositive(); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java index a0dc66f81f7..f8c6fa39235 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java @@ -101,7 +101,7 @@ void solveWithCustomMetrics() { ((DefaultSolver) solver).addPhaseLifecycleListener(new PhaseLifecycleListenerAdapter<>() { @Override public void solvingEnded(SolverScope solverScope) { - meterRegistry.publish(solver); + meterRegistry.publish(); var changeMoveKey = "ChangeMove(TestdataEntity.value)"; if (solverScope.getMoveCountTypes().contains(changeMoveKey)) { var counter = meterRegistry @@ -148,7 +148,7 @@ void solveWithListVariableAndCustomMetrics() { ((DefaultSolver) solver).addPhaseLifecycleListener(new PhaseLifecycleListenerAdapter<>() { @Override public void solvingEnded(SolverScope solverScope) { - meterRegistry.publish(solver); + meterRegistry.publish(); var changeMoveKey = "ListChangeMove(TestdataListEntity.valueList)"; if (solverScope.getMoveCountTypes().contains(changeMoveKey)) { var counter = meterRegistry diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index 92c6a7cf0ef..7fae39f1f63 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -94,6 +94,7 @@ import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.extension.ExtendWith; @@ -251,7 +252,7 @@ void checkDefaultMeters() { SolverFactory solverFactory = SolverFactory.create(solverConfig); var solver = (DefaultSolver) solverFactory.buildSolver(); - meterRegistry.publish(solver); + meterRegistry.publish(); assertThat(meterRegistry.getMeters().stream().map(Meter::getId)).isEmpty(); var solution = new TestdataSolution("s1"); @@ -338,7 +339,7 @@ void checkDefaultMetersTags() { var solver = (DefaultSolver) solverFactory.buildSolver(); solver.setMonitorTagMap(Map.of("tag.key", "tag.value")); - meterRegistry.publish(solver); + meterRegistry.publish(); assertThat(meterRegistry.getMeters().stream().map(Meter::getId)).isEmpty(); var solution = new TestdataSolution("s1"); @@ -425,7 +426,7 @@ void solveMetrics() { var solver = solverFactory.buildSolver(); ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", "solveMetrics")); - meterRegistry.publish(solver); + meterRegistry.publish(); var solution = new TestdataSolution("s1"); solution.setValueList(Arrays.asList(new TestdataValue("v1"), new TestdataValue("v2"))); @@ -435,7 +436,7 @@ void solveMetrics() { solver.addEventListener(event -> { if (!updatedTime.get()) { meterRegistry.getClock().addSeconds(2); - meterRegistry.publish(solver); + meterRegistry.publish(); assertThat(meterRegistry.getMeasurement(SolverMetric.SOLVE_DURATION.getMeterId(), "ACTIVE_TASKS")).isOne(); assertThat(meterRegistry.getMeasurement(SolverMetric.SOLVE_DURATION.getMeterId(), "DURATION").longValue()) .isEqualTo(2L); @@ -454,7 +455,7 @@ void solveMetrics() { }); solution = solver.solve(solution); - meterRegistry.publish(solver); + meterRegistry.publish(); assertThat(solution).isNotNull(); assertThat(solution.getScore().isSolutionInitialized()).isTrue(); @@ -475,7 +476,7 @@ void solveMetricsProblemChange() throws InterruptedException, ExecutionException SolverFactory solverFactory = SolverFactory.create(solverConfig); var solver = solverFactory.buildSolver(); - meterRegistry.publish(solver); + meterRegistry.publish(); final var solution = new TestdataSolution("s1"); solution.setValueList(new ArrayList<>(List.of(new TestdataValue("v1"), new TestdataValue("v2")))); @@ -485,7 +486,7 @@ void solveMetricsProblemChange() throws InterruptedException, ExecutionException solver.addEventListener(bestSolutionChangedEvent -> { try { latch.await(); - meterRegistry.publish(solver); + meterRegistry.publish(); } catch (InterruptedException e) { throw new RuntimeException(e); } @@ -565,14 +566,14 @@ void solveBestScoreMetrics() { var solver = solverFactory.buildSolver(); ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", "solveMetrics")); - meterRegistry.publish(solver); + meterRegistry.publish(); var solution = new TestdataHardSoftScoreSolution("s1"); solution.setValueList(Arrays.asList(new TestdataValue("none"), new TestdataValue("reward"))); solution.setEntityList(Arrays.asList(new TestdataEntity("e1"), new TestdataEntity("e2"))); var step = new AtomicInteger(-1); solver.addEventListener(event -> { - meterRegistry.publish(solver); + meterRegistry.publish(); // This event listener is added before the best score event listener // so it is one step behind @@ -599,7 +600,7 @@ void solveBestScoreMetrics() { solution = solver.solve(solution); assertThat(step.get()).isEqualTo(2); - meterRegistry.publish(solver); + meterRegistry.publish(); assertThat(solution).isNotNull(); assertThat(meterRegistry.getMeasurement(SolverMetric.BEST_SCORE.getMeterId() + ".hard.score", "VALUE").intValue()) .isEqualTo(0); @@ -676,7 +677,7 @@ void solveStepScoreMetrics() { @Override public void stepEnded(AbstractStepScope stepScope) { super.stepEnded(stepScope); - meterRegistry.publish(solver); + meterRegistry.publish(); // first 3 steps are construction heuristic steps and don't have a step score since it uninitialized if (step.get() < 2) { @@ -721,7 +722,7 @@ public void stepEnded(AbstractStepScope stepScope solution = solver.solve(solution); assertThat(step.get()).isEqualTo(7); - meterRegistry.publish(solver); + meterRegistry.publish(); assertThat(solution).isNotNull(); assertThat(meterRegistry.getMeasurement(SolverMetric.STEP_SCORE.getMeterId() + ".hard.score", "VALUE").intValue()) .isEqualTo(0); @@ -749,20 +750,20 @@ void solveMetricsError() { var solver = solverFactory.buildSolver(); ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", "solveMetricsError")); - meterRegistry.publish(solver); + meterRegistry.publish(); var solution = new TestdataSolution("s1"); solution.setValueList(Arrays.asList(new TestdataValue("v1"), new TestdataValue("v2"))); solution.setEntityList(Arrays.asList(new TestdataEntity("e1"), new TestdataEntity("e2"))); - meterRegistry.publish(solver); + meterRegistry.publish(); assertThatCode(() -> { solver.solve(solution); }).hasStackTraceContaining("Thrown exception in constraint provider"); meterRegistry.getClock().addSeconds(1); - meterRegistry.publish(solver); + meterRegistry.publish(); assertThat(meterRegistry.getMeasurement(SolverMetric.SOLVE_DURATION.getMeterId(), "ACTIVE_TASKS")).isZero(); assertThat(meterRegistry.getMeasurement(SolverMetric.SOLVE_DURATION.getMeterId(), "DURATION")).isZero(); assertThat(meterRegistry.getMeasurement(SolverMetric.ERROR_COUNT.getMeterId(), "COUNT")).isOne(); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/testutil/TestMeterRegistry.java b/core/src/test/java/ai/timefold/solver/core/impl/testutil/TestMeterRegistry.java index a49af7aa868..c8a16e241d7 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/testutil/TestMeterRegistry.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/testutil/TestMeterRegistry.java @@ -6,9 +6,6 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; -import ai.timefold.solver.core.api.solver.Solver; -import ai.timefold.solver.core.impl.solver.DefaultSolver; - import io.micrometer.core.instrument.MockClock; import io.micrometer.core.instrument.config.NamingConvention; import io.micrometer.core.instrument.simple.SimpleConfig; @@ -42,9 +39,8 @@ public BigDecimal getMeasurement(String key, String statistic) { } } - public void publish(Solver solver) { - DefaultSolver defaultSolver = (DefaultSolver) solver; - this.getMeters().stream().forEach(meter -> { + public void publish() { + this.getMeters().forEach(meter -> { final Map meterMeasurementMap = new HashMap<>(); String meterTags = ""; if (meter.getId().getTags().size() > 1) { From 57b2650f5308c5e43a7677efa5eb563b1e808b90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 12:36:40 +0100 Subject: [PATCH 09/14] Final fix? --- .../impl/statistic/BestScoreStatistic.java | 19 ++++++++++++------- .../core/impl/solver/DefaultSolverTest.java | 1 - 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/statistic/BestScoreStatistic.java b/core/src/main/java/ai/timefold/solver/core/impl/statistic/BestScoreStatistic.java index 181263bb6b7..3c126981242 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/statistic/BestScoreStatistic.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/statistic/BestScoreStatistic.java @@ -9,7 +9,6 @@ import ai.timefold.solver.core.api.solver.Solver; import ai.timefold.solver.core.api.solver.event.SolverEventListener; import ai.timefold.solver.core.config.solver.monitoring.SolverMetric; -import ai.timefold.solver.core.impl.score.definition.ScoreDefinition; import ai.timefold.solver.core.impl.solver.DefaultSolver; import io.micrometer.core.instrument.Tags; @@ -25,16 +24,22 @@ public void unregister(Solver solver) { if (listener != null) { solver.removeEventListener(listener); } - tagsToBestScoreMap.clear(); + tagsToBestScoreMap.remove(extractTags(solver)); + } + + private static Tags extractTags(Solver solver) { + var defaultSolver = (DefaultSolver) solver; + return defaultSolver.getSolverScope().getMonitoringTags(); } @Override public void register(Solver solver) { - DefaultSolver defaultSolver = (DefaultSolver) solver; - ScoreDefinition scoreDefinition = defaultSolver.getSolverScope().getScoreDefinition(); - SolverEventListener listener = event -> SolverMetric.registerScoreMetrics(SolverMetric.BEST_SCORE, - defaultSolver.getSolverScope().getMonitoringTags(), - scoreDefinition, tagsToBestScoreMap, event.getNewBestScore()); + var defaultSolver = (DefaultSolver) solver; + var scoreDefinition = defaultSolver.getSolverScope().getScoreDefinition(); + var tags = extractTags(solver); + SolverEventListener listener = + event -> SolverMetric.registerScoreMetrics(SolverMetric.BEST_SCORE, tags, scoreDefinition, tagsToBestScoreMap, + event.getNewBestScore()); solverToEventListenerMap.put(defaultSolver, listener); defaultSolver.addEventListener(listener); } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index 7fae39f1f63..9c7ea563dd2 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -94,7 +94,6 @@ import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.extension.ExtendWith; From d3feab64ab21c12c9be9ef81a7e9337cf9c32ef3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 12:48:22 +0100 Subject: [PATCH 10/14] Another fix --- ...DefaultConstructionHeuristicPhaseTest.java | 12 ++++++++++++ .../DefaultExhaustiveSearchPhaseTest.java | 13 +++++++++++++ .../generic/RuinRecreateMoveSelectorTest.java | 19 +++++++++++++++---- .../ListRuinRecreateMoveSelectorTest.java | 16 +++++++++++++--- .../DefaultLocalSearchPhaseTest.java | 13 +++++++++++++ .../core/impl/solver/DefaultSolverTest.java | 2 ++ 6 files changed, 68 insertions(+), 7 deletions(-) diff --git a/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java index 91d7cd4f2f2..7bc61f34158 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java @@ -41,12 +41,24 @@ import ai.timefold.solver.core.impl.testdata.util.PlannerTestUtils; import ai.timefold.solver.core.impl.testutil.TestMeterRegistry; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; class DefaultConstructionHeuristicPhaseTest { + @BeforeEach + @AfterEach + void resetGlobalRegistry() { + Metrics.globalRegistry.clear(); + List meterRegistryList = new ArrayList<>(); + meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); + meterRegistryList.forEach(Metrics.globalRegistry::remove); + } + @Test void solveWithInitializedEntities() { var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class) diff --git a/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java index 3c696b659ec..e78e739c06f 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java @@ -9,6 +9,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -44,12 +45,24 @@ import ai.timefold.solver.core.preview.api.move.Move; import ai.timefold.solver.core.preview.api.move.MutableSolutionView; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; class DefaultExhaustiveSearchPhaseTest { + @BeforeEach + @AfterEach + void resetGlobalRegistry() { + Metrics.globalRegistry.clear(); + List meterRegistryList = new ArrayList<>(); + meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); + meterRegistryList.forEach(Metrics.globalRegistry::remove); + } + @SuppressWarnings({ "unchecked", "rawtypes" }) @Test void restoreWorkingSolution() { diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java index 8a0efa51b5b..5665e102afc 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import java.util.ArrayList; import java.util.List; import ai.timefold.solver.core.api.solver.SolverFactory; @@ -21,15 +22,25 @@ import ai.timefold.solver.core.impl.testdata.domain.allows_unassigned.TestdataAllowsUnassignedSolution; import ai.timefold.solver.core.impl.testutil.TestMeterRegistry; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.parallel.Execution; -import org.junit.jupiter.api.parallel.ExecutionMode; +import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; -@Execution(ExecutionMode.CONCURRENT) class RuinRecreateMoveSelectorTest { + @BeforeEach + @AfterEach + void resetGlobalRegistry() { + Metrics.globalRegistry.clear(); + List meterRegistryList = new ArrayList<>(); + meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); + meterRegistryList.forEach(Metrics.globalRegistry::remove); + } + @Test void testRuining() { var solverConfig = new SolverConfig() @@ -48,7 +59,7 @@ void testRuining() { assertDoesNotThrow(() -> solver.solve(problem)); } - @Test + @RepeatedTest(2) void testRuiningWithMetric() { var meterRegistry = new TestMeterRegistry(); Metrics.addRegistry(meterRegistry); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java index 0213c69a1c0..906a363ce19 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.stream.IntStream; @@ -32,13 +33,13 @@ import ai.timefold.solver.core.impl.testutil.TestMeterRegistry; import org.jspecify.annotations.NonNull; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.parallel.Execution; -import org.junit.jupiter.api.parallel.ExecutionMode; +import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; -@Execution(ExecutionMode.CONCURRENT) class ListRuinRecreateMoveSelectorTest { public static final class TestdataListConstraintProvider implements ConstraintProvider { @@ -56,6 +57,15 @@ public static final class TestdataListConstraintProvider implements ConstraintPr } } + @BeforeEach + @AfterEach + void resetGlobalRegistry() { + Metrics.globalRegistry.clear(); + List meterRegistryList = new ArrayList<>(); + meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); + meterRegistryList.forEach(Metrics.globalRegistry::remove); + } + @Test void testRuining() { var solverConfig = new SolverConfig() diff --git a/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java index f8c6fa39235..39090def68b 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -37,12 +38,24 @@ import ai.timefold.solver.core.impl.testdata.util.PlannerTestUtils; import ai.timefold.solver.core.impl.testutil.TestMeterRegistry; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; class DefaultLocalSearchPhaseTest { + @BeforeEach + @AfterEach + void resetGlobalRegistry() { + Metrics.globalRegistry.clear(); + List meterRegistryList = new ArrayList<>(); + meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); + meterRegistryList.forEach(Metrics.globalRegistry::remove); + } + @Test void solveWithInitializedEntities() { var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index 9c7ea563dd2..b9d8eefdd1f 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -93,6 +93,7 @@ import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; @@ -107,6 +108,7 @@ class DefaultSolverTest { @BeforeEach + @AfterEach void resetGlobalRegistry() { Metrics.globalRegistry.clear(); List meterRegistryList = new ArrayList<>(); From 08d5e1f546198e1c1ebd981a583522dbe7458902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 13:09:21 +0100 Subject: [PATCH 11/14] Attempt more fixes --- .../move/generic/RuinRecreateMoveSelectorTest.java | 3 +-- .../solver/core/impl/solver/DefaultSolverTest.java | 14 +++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java index 5665e102afc..2755e8bd98d 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java @@ -24,7 +24,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import io.micrometer.core.instrument.MeterRegistry; @@ -59,7 +58,7 @@ void testRuining() { assertDoesNotThrow(() -> solver.solve(problem)); } - @RepeatedTest(2) + @Test void testRuiningWithMetric() { var meterRegistry = new TestMeterRegistry(); Metrics.addRegistry(meterRegistry); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index b9d8eefdd1f..68e81cfb17c 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -11,6 +11,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.UUID; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; @@ -426,7 +427,7 @@ void solveMetrics() { SolverFactory solverFactory = SolverFactory.create(solverConfig); var solver = solverFactory.buildSolver(); - ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", "solveMetrics")); + ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", UUID.randomUUID().toString())); meterRegistry.publish(); var solution = new TestdataSolution("s1"); @@ -566,7 +567,7 @@ void solveBestScoreMetrics() { SolverFactory solverFactory = SolverFactory.create(solverConfig); var solver = solverFactory.buildSolver(); - ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", "solveMetrics")); + ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", UUID.randomUUID().toString())); meterRegistry.publish(); var solution = new TestdataHardSoftScoreSolution("s1"); solution.setValueList(Arrays.asList(new TestdataValue("none"), new TestdataValue("reward"))); @@ -670,7 +671,7 @@ void solveStepScoreMetrics() { SolverFactory solverFactory = SolverFactory.create(solverConfig); var solver = solverFactory.buildSolver(); - ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", "solveMetrics")); + ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", UUID.randomUUID().toString())); var step = new AtomicInteger(-1); ((DefaultSolver) solver) @@ -750,7 +751,7 @@ void solveMetricsError() { SolverFactory solverFactory = SolverFactory.create(solverConfig); var solver = solverFactory.buildSolver(); - ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", "solveMetricsError")); + ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", UUID.randomUUID().toString())); meterRegistry.publish(); var solution = new TestdataSolution("s1"); @@ -759,9 +760,8 @@ void solveMetricsError() { meterRegistry.publish(); - assertThatCode(() -> { - solver.solve(solution); - }).hasStackTraceContaining("Thrown exception in constraint provider"); + assertThatCode(() -> solver.solve(solution)) + .hasStackTraceContaining("Thrown exception in constraint provider"); meterRegistry.getClock().addSeconds(1); meterRegistry.publish(); From d21cf9e76bbf26aedd16b653877206e38c96be12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 13:11:57 +0100 Subject: [PATCH 12/14] Formatting --- .../timefold/solver/core/impl/solver/DefaultSolverTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index 68e81cfb17c..f21c4262e1a 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -567,7 +567,8 @@ void solveBestScoreMetrics() { SolverFactory solverFactory = SolverFactory.create(solverConfig); var solver = solverFactory.buildSolver(); - ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", UUID.randomUUID().toString())); + ((DefaultSolver) solver) + .setMonitorTagMap(Map.of("solver.id", UUID.randomUUID().toString())); meterRegistry.publish(); var solution = new TestdataHardSoftScoreSolution("s1"); solution.setValueList(Arrays.asList(new TestdataValue("none"), new TestdataValue("reward"))); @@ -671,7 +672,8 @@ void solveStepScoreMetrics() { SolverFactory solverFactory = SolverFactory.create(solverConfig); var solver = solverFactory.buildSolver(); - ((DefaultSolver) solver).setMonitorTagMap(Map.of("solver.id", UUID.randomUUID().toString())); + ((DefaultSolver) solver) + .setMonitorTagMap(Map.of("solver.id", UUID.randomUUID().toString())); var step = new AtomicInteger(-1); ((DefaultSolver) solver) From ae5f349471dcc9aa40858708ccc976b15c464f01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 13:31:06 +0100 Subject: [PATCH 13/14] Attempt to fix the issues with latches --- .../core/impl/solver/DefaultSolverTest.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index f21c4262e1a..19cda0d581c 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -15,6 +15,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -262,6 +263,7 @@ void checkDefaultMeters() { solution.setEntityList(Arrays.asList(new TestdataEntity("e1"), new TestdataEntity("e2"))); var updatedTime = new AtomicBoolean(); + var latch = new CountDownLatch(1); solver.addEventListener(event -> { if (!updatedTime.get()) { assertThat(meterRegistry.getMeters().stream().map(Meter::getId)) @@ -308,9 +310,16 @@ void checkDefaultMeters() { Meter.Type.GAUGE)); updatedTime.set(true); } + latch.countDown(); }); solver.solve(solution); + try { + latch.await(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Assertions.fail("Failed waiting for the event to happen.", e); + } + // Score calculation and problem scale counts should be removed // since registering multiple gauges with the same id // make it return the average, and the solver holds @@ -349,6 +358,7 @@ void checkDefaultMetersTags() { solution.setEntityList(Arrays.asList(new TestdataEntity("e1"), new TestdataEntity("e2"))); var updatedTime = new AtomicBoolean(); + var latch = new CountDownLatch(1); solver.addEventListener(event -> { if (!updatedTime.get()) { assertThat(meterRegistry.getMeters().stream().map(Meter::getId)) @@ -395,9 +405,16 @@ void checkDefaultMetersTags() { Meter.Type.GAUGE)); updatedTime.set(true); } + latch.countDown(); }); solver.solve(solution); + try { + latch.await(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Assertions.fail("Failed waiting for the event to happen.", e); + } + // Score calculation and problem scale counts should be removed // since registering multiple gauges with the same id // make it return the average, and the solver holds @@ -434,6 +451,7 @@ void solveMetrics() { solution.setValueList(Arrays.asList(new TestdataValue("v1"), new TestdataValue("v2"))); solution.setEntityList(Arrays.asList(new TestdataEntity("e1"), new TestdataEntity("e2"))); + var latch = new CountDownLatch(1); var updatedTime = new AtomicBoolean(); solver.addEventListener(event -> { if (!updatedTime.get()) { @@ -454,9 +472,15 @@ void solveMetrics() { .isEqualTo(2L); updatedTime.set(true); } + latch.countDown(); }); solution = solver.solve(solution); + try { + latch.await(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Assertions.fail("Failed waiting for the event to happen.", e); + } meterRegistry.publish(); assertThat(solution).isNotNull(); assertThat(solution.getScore().isSolutionInitialized()).isTrue(); @@ -575,6 +599,7 @@ void solveBestScoreMetrics() { solution.setEntityList(Arrays.asList(new TestdataEntity("e1"), new TestdataEntity("e2"))); var step = new AtomicInteger(-1); + var latch = new CountDownLatch(1); solver.addEventListener(event -> { meterRegistry.publish(); @@ -599,9 +624,15 @@ void solveBestScoreMetrics() { .isEqualTo(2); } step.incrementAndGet(); + latch.countDown(); }); solution = solver.solve(solution); + try { + latch.await(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { + fail("Failed waiting for the event to happen.", e); + } assertThat(step.get()).isEqualTo(2); meterRegistry.publish(); assertThat(solution).isNotNull(); From b501ddd1d960e37e458251927f5a339f30307f39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 25 Feb 2025 14:56:43 +0100 Subject: [PATCH 14/14] Finishing touches --- ...DefaultConstructionHeuristicPhaseTest.java | 16 +--- .../DefaultExhaustiveSearchPhaseTest.java | 17 +--- .../generic/RuinRecreateMoveSelectorTest.java | 17 +--- .../ListRuinRecreateMoveSelectorTest.java | 17 +--- .../DefaultLocalSearchPhaseTest.java | 17 +--- .../core/impl/solver/DefaultSolverTest.java | 19 +--- .../core/impl/testutil/AbstractMeterTest.java | 88 +++++++++++++++++++ .../core/impl/testutil/TestMeterRegistry.java | 67 -------------- 8 files changed, 101 insertions(+), 157 deletions(-) create mode 100644 core/src/test/java/ai/timefold/solver/core/impl/testutil/AbstractMeterTest.java delete mode 100644 core/src/test/java/ai/timefold/solver/core/impl/testutil/TestMeterRegistry.java diff --git a/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java index 7bc61f34158..fcecda40d81 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java @@ -39,25 +39,13 @@ import ai.timefold.solver.core.impl.testdata.domain.pinned.allows_unassigned.TestdataPinnedAllowsUnassignedEntity; import ai.timefold.solver.core.impl.testdata.domain.pinned.allows_unassigned.TestdataPinnedAllowsUnassignedSolution; import ai.timefold.solver.core.impl.testdata.util.PlannerTestUtils; -import ai.timefold.solver.core.impl.testutil.TestMeterRegistry; +import ai.timefold.solver.core.impl.testutil.AbstractMeterTest; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; -class DefaultConstructionHeuristicPhaseTest { - - @BeforeEach - @AfterEach - void resetGlobalRegistry() { - Metrics.globalRegistry.clear(); - List meterRegistryList = new ArrayList<>(); - meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); - meterRegistryList.forEach(Metrics.globalRegistry::remove); - } +class DefaultConstructionHeuristicPhaseTest extends AbstractMeterTest { @Test void solveWithInitializedEntities() { diff --git a/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java index e78e739c06f..168616bf878 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java @@ -9,7 +9,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -41,27 +40,15 @@ import ai.timefold.solver.core.impl.testdata.domain.pinned.allows_unassigned.TestdataPinnedAllowsUnassignedEntity; import ai.timefold.solver.core.impl.testdata.domain.pinned.allows_unassigned.TestdataPinnedAllowsUnassignedSolution; import ai.timefold.solver.core.impl.testdata.util.PlannerTestUtils; -import ai.timefold.solver.core.impl.testutil.TestMeterRegistry; +import ai.timefold.solver.core.impl.testutil.AbstractMeterTest; import ai.timefold.solver.core.preview.api.move.Move; import ai.timefold.solver.core.preview.api.move.MutableSolutionView; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; -class DefaultExhaustiveSearchPhaseTest { - - @BeforeEach - @AfterEach - void resetGlobalRegistry() { - Metrics.globalRegistry.clear(); - List meterRegistryList = new ArrayList<>(); - meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); - meterRegistryList.forEach(Metrics.globalRegistry::remove); - } +class DefaultExhaustiveSearchPhaseTest extends AbstractMeterTest { @SuppressWarnings({ "unchecked", "rawtypes" }) @Test diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java index 2755e8bd98d..55a565ce0ae 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/RuinRecreateMoveSelectorTest.java @@ -3,7 +3,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import java.util.ArrayList; import java.util.List; import ai.timefold.solver.core.api.solver.SolverFactory; @@ -20,25 +19,13 @@ import ai.timefold.solver.core.impl.testdata.domain.allows_unassigned.TestdataAllowsUnassignedEasyScoreCalculator; import ai.timefold.solver.core.impl.testdata.domain.allows_unassigned.TestdataAllowsUnassignedEntity; import ai.timefold.solver.core.impl.testdata.domain.allows_unassigned.TestdataAllowsUnassignedSolution; -import ai.timefold.solver.core.impl.testutil.TestMeterRegistry; +import ai.timefold.solver.core.impl.testutil.AbstractMeterTest; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; -class RuinRecreateMoveSelectorTest { - - @BeforeEach - @AfterEach - void resetGlobalRegistry() { - Metrics.globalRegistry.clear(); - List meterRegistryList = new ArrayList<>(); - meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); - meterRegistryList.forEach(Metrics.globalRegistry::remove); - } +class RuinRecreateMoveSelectorTest extends AbstractMeterTest { @Test void testRuining() { diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java index 906a363ce19..8f2dbf3d449 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListRuinRecreateMoveSelectorTest.java @@ -3,7 +3,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.stream.IntStream; @@ -30,17 +29,14 @@ import ai.timefold.solver.core.impl.testdata.domain.list.allows_unassigned.TestdataAllowsUnassignedValuesListEntity; import ai.timefold.solver.core.impl.testdata.domain.list.allows_unassigned.TestdataAllowsUnassignedValuesListSolution; import ai.timefold.solver.core.impl.testdata.domain.list.allows_unassigned.TestdataAllowsUnassignedValuesListValue; -import ai.timefold.solver.core.impl.testutil.TestMeterRegistry; +import ai.timefold.solver.core.impl.testutil.AbstractMeterTest; import org.jspecify.annotations.NonNull; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; -class ListRuinRecreateMoveSelectorTest { +class ListRuinRecreateMoveSelectorTest extends AbstractMeterTest { public static final class TestdataListConstraintProvider implements ConstraintProvider { @@ -57,15 +53,6 @@ public static final class TestdataListConstraintProvider implements ConstraintPr } } - @BeforeEach - @AfterEach - void resetGlobalRegistry() { - Metrics.globalRegistry.clear(); - List meterRegistryList = new ArrayList<>(); - meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); - meterRegistryList.forEach(Metrics.globalRegistry::remove); - } - @Test void testRuining() { var solverConfig = new SolverConfig() diff --git a/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java index 39090def68b..e815056b7dc 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java @@ -4,7 +4,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -36,25 +35,13 @@ import ai.timefold.solver.core.impl.testdata.domain.pinned.allows_unassigned.TestdataPinnedAllowsUnassignedEntity; import ai.timefold.solver.core.impl.testdata.domain.pinned.allows_unassigned.TestdataPinnedAllowsUnassignedSolution; import ai.timefold.solver.core.impl.testdata.util.PlannerTestUtils; -import ai.timefold.solver.core.impl.testutil.TestMeterRegistry; +import ai.timefold.solver.core.impl.testutil.AbstractMeterTest; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; -class DefaultLocalSearchPhaseTest { - - @BeforeEach - @AfterEach - void resetGlobalRegistry() { - Metrics.globalRegistry.clear(); - List meterRegistryList = new ArrayList<>(); - meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); - meterRegistryList.forEach(Metrics.globalRegistry::remove); - } +class DefaultLocalSearchPhaseTest extends AbstractMeterTest { @Test void solveWithInitializedEntities() { diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index 19cda0d581c..90c4f14e30d 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -88,35 +88,23 @@ import ai.timefold.solver.core.impl.testdata.domain.pinned.TestdataPinnedSolution; import ai.timefold.solver.core.impl.testdata.domain.score.TestdataHardSoftScoreSolution; import ai.timefold.solver.core.impl.testdata.util.PlannerTestUtils; -import ai.timefold.solver.core.impl.testutil.TestMeterRegistry; +import ai.timefold.solver.core.impl.testutil.AbstractMeterTest; import org.assertj.core.api.Assertions; import org.assertj.core.api.SoftAssertions; import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.extension.ExtendWith; import io.micrometer.core.instrument.Meter; -import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.Tags; @ExtendWith(SoftAssertionsExtension.class) -class DefaultSolverTest { - - @BeforeEach - @AfterEach - void resetGlobalRegistry() { - Metrics.globalRegistry.clear(); - List meterRegistryList = new ArrayList<>(); - meterRegistryList.addAll(Metrics.globalRegistry.getRegistries()); - meterRegistryList.forEach(Metrics.globalRegistry::remove); - } +class DefaultSolverTest extends AbstractMeterTest { @Test void solve() { @@ -344,8 +332,7 @@ void checkDefaultMetersTags() { var meterRegistry = new TestMeterRegistry(); Metrics.addRegistry(meterRegistry); - var solverConfig = PlannerTestUtils.buildSolverConfig( - TestdataSolution.class, TestdataEntity.class); + var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class); SolverFactory solverFactory = SolverFactory.create(solverConfig); var solver = (DefaultSolver) solverFactory.buildSolver(); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/testutil/AbstractMeterTest.java b/core/src/test/java/ai/timefold/solver/core/impl/testutil/AbstractMeterTest.java new file mode 100644 index 00000000000..36df9b96ff0 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/impl/testutil/AbstractMeterTest.java @@ -0,0 +1,88 @@ +package ai.timefold.solver.core.impl.testutil; + +import java.math.BigDecimal; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; + +import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.MockClock; +import io.micrometer.core.instrument.config.NamingConvention; +import io.micrometer.core.instrument.simple.SimpleConfig; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; + +public abstract class AbstractMeterTest { + + @BeforeEach // To guard against nasty tests which do not do this. + @AfterEach // To clean up after ourselves. + void resetGlobalRegistry() { + List.copyOf(Metrics.globalRegistry.getRegistries()) + .forEach(registry -> { + Metrics.globalRegistry.remove(registry); + registry.clear(); + }); + Metrics.globalRegistry.clear(); // Make absolutely sure that the global registry is empty. + } + + protected static final class TestMeterRegistry extends SimpleMeterRegistry { + + private final Map> measurementMap; + + public TestMeterRegistry() { + super(SimpleConfig.DEFAULT, new MockClock()); + measurementMap = new LinkedHashMap<>(); + } + + public MockClock getClock() { + return MockClock.clock(this); + } + + public BigDecimal getMeasurement(String key, String statistic) { + if (measurementMap.containsKey(key)) { + Map meterMeasurementMap = measurementMap.get(key); + if (meterMeasurementMap.containsKey(statistic)) { + return meterMeasurementMap.get(statistic); + } else { + throw new IllegalArgumentException( + "Meter (" + key + ") does not have statistic (" + statistic + "). Available statistics are: " + + meterMeasurementMap.keySet().stream().collect(Collectors.joining(", ", "[", "]"))); + } + } else { + throw new IllegalArgumentException("Meter (" + key + ") does not exist. Available statistics are: " + + measurementMap.keySet().stream().collect(Collectors.joining(", ", "[", "]"))); + } + } + + public void publish() { + this.getMeters().forEach(meter -> { + final Map meterMeasurementMap = new LinkedHashMap<>(); + String meterTags = ""; + if (meter.getId().getTags().size() > 1) { + meterTags = meter.getId().getConventionTags(NamingConvention.dot).stream() + .filter(tag -> !tag.getKey().equals("solver.id")) + .map(tag -> tag.getKey() + "=" + tag.getValue()) + .sorted() + .collect(Collectors.joining(",", ":", "")); + } + measurementMap.put(meter.getId().getConventionName(NamingConvention.dot) + meterTags, + meterMeasurementMap); + meter.measure().forEach(measurement -> { + if (Double.isFinite(measurement.getValue())) { + meterMeasurementMap.put(measurement.getStatistic().name(), BigDecimal.valueOf(measurement.getValue())); + } + }); + }); + } + + @Override + protected TimeUnit getBaseTimeUnit() { + return TimeUnit.SECONDS; + } + } + +} diff --git a/core/src/test/java/ai/timefold/solver/core/impl/testutil/TestMeterRegistry.java b/core/src/test/java/ai/timefold/solver/core/impl/testutil/TestMeterRegistry.java deleted file mode 100644 index c8a16e241d7..00000000000 --- a/core/src/test/java/ai/timefold/solver/core/impl/testutil/TestMeterRegistry.java +++ /dev/null @@ -1,67 +0,0 @@ -package ai.timefold.solver.core.impl.testutil; - -import java.math.BigDecimal; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; - -import io.micrometer.core.instrument.MockClock; -import io.micrometer.core.instrument.config.NamingConvention; -import io.micrometer.core.instrument.simple.SimpleConfig; -import io.micrometer.core.instrument.simple.SimpleMeterRegistry; - -public class TestMeterRegistry extends SimpleMeterRegistry { - final Map> measurementMap; - - public TestMeterRegistry() { - super(SimpleConfig.DEFAULT, new MockClock()); - measurementMap = new HashMap<>(); - } - - public MockClock getClock() { - return MockClock.clock(this); - } - - public BigDecimal getMeasurement(String key, String statistic) { - if (measurementMap.containsKey(key)) { - Map meterMeasurementMap = measurementMap.get(key); - if (meterMeasurementMap.containsKey(statistic)) { - return meterMeasurementMap.get(statistic); - } else { - throw new IllegalArgumentException( - "Meter (" + key + ") does not have statistic (" + statistic + "). Available statistics are: " - + meterMeasurementMap.keySet().stream().collect(Collectors.joining(", ", "[", "]"))); - } - } else { - throw new IllegalArgumentException("Meter (" + key + ") does not exist. Available statistics are: " - + measurementMap.keySet().stream().collect(Collectors.joining(", ", "[", "]"))); - } - } - - public void publish() { - this.getMeters().forEach(meter -> { - final Map meterMeasurementMap = new HashMap<>(); - String meterTags = ""; - if (meter.getId().getTags().size() > 1) { - meterTags = meter.getId().getConventionTags(NamingConvention.dot).stream() - .filter(tag -> !tag.getKey().equals("solver.id")) - .map(tag -> tag.getKey() + "=" + tag.getValue()) - .sorted() - .collect(Collectors.joining(",", ":", "")); - } - measurementMap.put(meter.getId().getConventionName(NamingConvention.dot) + meterTags, - meterMeasurementMap); - meter.measure().forEach(measurement -> { - if (Double.isFinite(measurement.getValue())) { - meterMeasurementMap.put(measurement.getStatistic().name(), BigDecimal.valueOf(measurement.getValue())); - } - }); - }); - } - - @Override - protected TimeUnit getBaseTimeUnit() { - return TimeUnit.SECONDS; - } -}