diff --git a/core/src/main/java/ai/timefold/solver/core/api/domain/variable/InconsistentSolutionException.java b/core/src/main/java/ai/timefold/solver/core/api/domain/variable/InconsistentSolutionException.java new file mode 100644 index 00000000000..96039b27321 --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/api/domain/variable/InconsistentSolutionException.java @@ -0,0 +1,27 @@ +package ai.timefold.solver.core.api.domain.variable; + +import java.util.List; + +import org.jspecify.annotations.NullMarked; + +@NullMarked +public class InconsistentSolutionException extends RuntimeException { + private final Object solution; + private final List involvedEntityList; + + public InconsistentSolutionException(String feature, Object solution, List involvedEntityList) { + super("The solution (%s) is inconsistent. %s requires a consistent solution.".formatted(solution, feature)); + this.solution = solution; + this.involvedEntityList = involvedEntityList; + } + + @SuppressWarnings("unchecked") + public T getSolution() { + return (T) solution; + } + + @SuppressWarnings("unchecked") + public List getInvolvedEntityList() { + return (List) involvedEntityList; + } +} 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 f166726d9f5..73d2c0f1ca6 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 @@ -75,7 +75,7 @@ public abstract class AbstractScoreDirector variableDescriptorCache; protected final VariableListenerSupport variableListenerSupport; private final @Nullable SolutionTracker solutionTracker; // Null when tracking disabled. @@ -111,7 +111,7 @@ protected AbstractScoreDirector(AbstractScoreDirectorBuilder(solutionDescriptor); this.variableListenerSupport = VariableListenerSupport.create(this); this.variableListenerSupport.linkVariableListeners(); @@ -344,8 +344,12 @@ protected void afterSetWorkingSolution() { // Do nothing } + public List getInconsistentEntities() { + return variableListenerSupport.getInconsistentEntities(); + } + public void unassignInconsistentEntities() { - var inconsistentEntities = variableListenerSupport.getInconsistentEntities(); + var inconsistentEntities = getInconsistentEntities(); if (listVariableStateSupply != null) { var listVariableDescriptor = listVariableStateSupply.getSourceVariableDescriptor(); var listElementClass = listVariableStateSupply.getSourceVariableDescriptor().getElementType(); @@ -490,6 +494,10 @@ public void triggerVariableListeners() { lastVariableUpdateWasSuccessful = variableListenerSupport.triggerVariableListenersInNotificationQueues(); } + public boolean isLastVariableUpdateWasSuccessful() { + return lastVariableUpdateWasSuccessful; + } + /** * This function clears all listener events that have been generated without triggering any of them. * Using this method requires caution because clearing the event queue can lead to inconsistent states. @@ -517,7 +525,6 @@ public InnerScoreDirector createChildThreadScoreDirector(Chil var childThreadScoreDirector = scoreDirectorFactory.createScoreDirectorBuilder() .withLookUpEnabled(lookUpEnabled) .withConstraintMatchPolicy(constraintMatchPolicy) - .withIgnoreInconsistentSolutions(ignoreInconsistentSolutions) .buildDerived(); // ScoreCalculationCountTermination takes into account previous phases // but the calculationCount of partitions is maxed, not summed. @@ -527,7 +534,6 @@ public InnerScoreDirector createChildThreadScoreDirector(Chil var childThreadScoreDirector = scoreDirectorFactory.createScoreDirectorBuilder() .withLookUpEnabled(true) .withConstraintMatchPolicy(constraintMatchPolicy) - .withIgnoreInconsistentSolutions(ignoreInconsistentSolutions) .buildDerived(); childThreadScoreDirector.setWorkingSolution(cloneWorkingSolution()); return childThreadScoreDirector; @@ -800,7 +806,6 @@ private void assertScoreFromScratch(InnerScore innerScore, Object comple // Most score directors don't need derived status; CS will override this. try (var uncorruptedScoreDirector = assertionScoreDirectorFactory.createScoreDirectorBuilder() .withConstraintMatchPolicy(ConstraintMatchPolicy.ENABLED) - .withIgnoreInconsistentSolutions(ignoreInconsistentSolutions) .buildDerived()) { uncorruptedScoreDirector.setWorkingSolution(workingSolution); var uncorruptedInnerScore = uncorruptedScoreDirector.calculateScore(); @@ -999,7 +1004,6 @@ public abstract static class AbstractScoreDirectorBuilder build(); /** diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java index 74d8e95bf02..06c53bf2a25 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java @@ -5,6 +5,7 @@ import java.util.function.Function; import ai.timefold.solver.core.api.domain.solution.PlanningSolution; +import ai.timefold.solver.core.api.domain.variable.InconsistentSolutionException; import ai.timefold.solver.core.api.score.Score; import ai.timefold.solver.core.api.score.analysis.ScoreAnalysis; import ai.timefold.solver.core.api.solver.RecommendedAssignment; @@ -51,14 +52,15 @@ public ScoreDirectorFactory getScoreDirectorFactory() { @Override public Score_ update(Solution_ solution, SolutionUpdatePolicy solutionUpdatePolicy) { if (solutionUpdatePolicy == SolutionUpdatePolicy.NO_UPDATE) { - throw new IllegalArgumentException("Can not call " + this.getClass().getSimpleName() - + ".update() with this solutionUpdatePolicy (" + solutionUpdatePolicy + ")."); + throw new IllegalArgumentException( + "Can not call %s.update() with this solutionUpdatePolicy (%s)." + .formatted(this.getClass().getSimpleName(), solutionUpdatePolicy)); } - return callScoreDirector(solution, solutionUpdatePolicy, + return callScoreDirector("Solution update", solution, solutionUpdatePolicy, s -> s.getSolutionDescriptor().getScore(s.getWorkingSolution()), ConstraintMatchPolicy.DISABLED, false); } - private Result_ callScoreDirector(Solution_ solution, SolutionUpdatePolicy solutionUpdatePolicy, + private Result_ callScoreDirector(String feature, Solution_ solution, SolutionUpdatePolicy solutionUpdatePolicy, Function, Result_> function, ConstraintMatchPolicy constraintMatchPolicy, boolean cloneSolution) { var isShadowVariableUpdateEnabled = solutionUpdatePolicy.isShadowVariableUpdateEnabled(); @@ -82,7 +84,14 @@ private Result_ callScoreDirector(Solution_ solution, SolutionUpdatePo Maybe use Constraint Streams instead of Easy or Incremental score calculator?"""); } if (solutionUpdatePolicy.isScoreUpdateEnabled()) { - scoreDirector.calculateScore(); + var score = scoreDirector.calculateScore(); + if (score.isInvalid()) { + var inconsistentEntities = scoreDirector.getInconsistentEntities(); + throw new InconsistentSolutionException(feature, nonNullSolution, inconsistentEntities); + } + } else if (!scoreDirector.isLastVariableUpdateWasSuccessful()) { + var inconsistentEntities = scoreDirector.getInconsistentEntities(); + throw new InconsistentSolutionException(feature, nonNullSolution, inconsistentEntities); } return function.apply(scoreDirector); } @@ -112,7 +121,7 @@ public ScoreAnalysis analyze(Solution_ solution, ScoreAnalysisFetchPolic var enterpriseService = TimefoldSolverEnterpriseService.loadOrFail(TimefoldSolverEnterpriseService.Feature.SCORE_ANALYSIS); var currentScore = (Score_) scoreDirectorFactory.getSolutionDescriptor().getScore(solution); - var analysis = callScoreDirector(solution, solutionUpdatePolicy, + var analysis = callScoreDirector("Solution analysis", solution, solutionUpdatePolicy, scoreDirector -> enterpriseService.analyze(scoreDirector.calculateScore(), scoreDirector.getConstraintMatchTotalMap(), fetchPolicy), ConstraintMatchPolicy.match(fetchPolicy), false); @@ -134,7 +143,7 @@ public List> recommendAssignment ScoreAnalysisFetchPolicy fetchPolicy) { var enterpriseService = TimefoldSolverEnterpriseService.loadOrFail(TimefoldSolverEnterpriseService.Feature.RECOMMENDATIONS); - return callScoreDirector( + return callScoreDirector("Recommended assignment", solution, SolutionUpdatePolicy.UPDATE_ALL, enterpriseService.buildRecommender(solverFactory, solution, evaluatedEntityOrElement, propositionFunction, fetchPolicy), ConstraintMatchPolicy.match(fetchPolicy), true); 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 1e718b36475..8d841424c32 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 @@ -125,7 +125,6 @@ public Solver buildSolver(SolverConfigOverride configOverride) { } var castScoreDirector = scoreDirectorFactory.createScoreDirectorBuilder() .withLookUpEnabled(true) // Custom phases and problem changes may rely on lookups. - .withIgnoreInconsistentSolutions(!solutionDescriptor.hasAnyShadowVariablesInconsistentMember()) .withConstraintMatchPolicy( constraintMatchEnabled ? ConstraintMatchPolicy.ENABLED : ConstraintMatchPolicy.DISABLED) .build(); diff --git a/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java b/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java index d2405c4b11d..d5b31dd39cc 100644 --- a/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java +++ b/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java @@ -4,10 +4,12 @@ import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.SoftAssertions.assertSoftly; +import java.time.Duration; import java.util.Arrays; import java.util.List; import java.util.function.Function; +import ai.timefold.solver.core.api.domain.variable.InconsistentSolutionException; import ai.timefold.solver.core.api.score.HardSoftScore; import ai.timefold.solver.core.api.score.Score; import ai.timefold.solver.core.config.score.director.ScoreDirectorFactoryConfig; @@ -23,6 +25,10 @@ import ai.timefold.solver.core.testdomain.shadow.concurrent.TestdataConcurrentEntity; import ai.timefold.solver.core.testdomain.shadow.concurrent.TestdataConcurrentSolution; import ai.timefold.solver.core.testdomain.shadow.concurrent.TestdataConcurrentValue; +import ai.timefold.solver.core.testdomain.shadow.no_inconsistent_field.TestdataDependencyNoInconsistentFieldConstraintProvider; +import ai.timefold.solver.core.testdomain.shadow.no_inconsistent_field.TestdataDependencyNoInconsistentFieldEntity; +import ai.timefold.solver.core.testdomain.shadow.no_inconsistent_field.TestdataDependencyNoInconsistentFieldSolution; +import ai.timefold.solver.core.testdomain.shadow.no_inconsistent_field.TestdataDependencyNoInconsistentFieldValue; import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; @@ -113,6 +119,32 @@ void updateEverythingList(SolutionManagerSource solutionManagerSource) { }); } + @ParameterizedTest + @EnumSource(SolutionManagerSource.class) + void updateInconsistent(SolutionManagerSource solutionManagerSource) { + var entity1 = new TestdataDependencyNoInconsistentFieldEntity("e1"); + var valueA1 = new TestdataDependencyNoInconsistentFieldValue("a1"); + var valueA2 = new TestdataDependencyNoInconsistentFieldValue("a2", Duration.ofHours(1L), List.of(valueA1)); + entity1.setValues(List.of(valueA2, valueA1)); + var inconsistentSolution = + new TestdataDependencyNoInconsistentFieldSolution(List.of(entity1), List.of(valueA1, valueA2)); + + var solverFactory = SolverFactory.create(new SolverConfig() + .withSolutionClass(TestdataDependencyNoInconsistentFieldSolution.class) + .withEntityClasses(TestdataDependencyNoInconsistentFieldEntity.class, + TestdataDependencyNoInconsistentFieldValue.class) + .withConstraintProviderClass(TestdataDependencyNoInconsistentFieldConstraintProvider.class)); + var solutionManager = solutionManagerSource.createSolutionManager(solverFactory); + assertThat(solutionManager).isNotNull(); + + assertThatCode(() -> solutionManager.update(inconsistentSolution)) + .isInstanceOf(InconsistentSolutionException.class) + .hasMessageContainingAll("The solution (", + "is inconsistent", "Solution update", "requires a consistent solution") + .hasFieldOrPropertyWithValue("solution", inconsistentSolution) + .hasFieldOrPropertyWithValue("involvedEntityList", List.of(valueA1, valueA2)); + } + private void assertShadowedListValueAllNull(SoftAssertions softly, TestdataListValueWithShadowHistory current) { softly.assertThat(current.getIndex()).isNull(); softly.assertThat(current.getEntity()).isNull(); @@ -149,6 +181,32 @@ void updateOnlyShadowVariables(SolutionManagerSource solutionManagerSource) { }); } + @ParameterizedTest + @EnumSource(SolutionManagerSource.class) + void updateOnlyShadowVariablesInconsistent(SolutionManagerSource solutionManagerSource) { + var entity1 = new TestdataDependencyNoInconsistentFieldEntity("e1"); + var valueA1 = new TestdataDependencyNoInconsistentFieldValue("a1"); + var valueA2 = new TestdataDependencyNoInconsistentFieldValue("a2", Duration.ofHours(1L), List.of(valueA1)); + entity1.setValues(List.of(valueA2, valueA1)); + var inconsistentSolution = + new TestdataDependencyNoInconsistentFieldSolution(List.of(entity1), List.of(valueA1, valueA2)); + + var solverFactory = SolverFactory.create(new SolverConfig() + .withSolutionClass(TestdataDependencyNoInconsistentFieldSolution.class) + .withEntityClasses(TestdataDependencyNoInconsistentFieldEntity.class, + TestdataDependencyNoInconsistentFieldValue.class) + .withConstraintProviderClass(TestdataDependencyNoInconsistentFieldConstraintProvider.class)); + var solutionManager = solutionManagerSource.createSolutionManager(solverFactory); + assertThat(solutionManager).isNotNull(); + + assertThatCode(() -> solutionManager.update(inconsistentSolution)) + .isInstanceOf(InconsistentSolutionException.class) + .hasMessageContainingAll("The solution (", + "is inconsistent", "Solution update", "requires a consistent solution") + .hasFieldOrPropertyWithValue("solution", inconsistentSolution) + .hasFieldOrPropertyWithValue("involvedEntityList", List.of(valueA1, valueA2)); + } + @ParameterizedTest @EnumSource(SolutionManagerSource.class) void updateOnlyScore(SolutionManagerSource solutionManagerSource) {