From e48cff21a8bb05df410da6279f4a12374275a787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Thu, 7 Nov 2024 13:49:51 +0100 Subject: [PATCH] Finishing touches --- .../score/analysis/ConstraintAnalysis.java | 5 ++-- .../score/stream/ConstraintJustification.java | 2 +- .../constraint/ConstraintMatchPolicy.java | 19 ++++++++++++ .../AbstractScoreDirectorFactory.java | 5 ---- .../director/InnerScoreDirectorFactory.java | 29 ++----------------- .../score/director/ScoreDirectorFactory.java | 15 ++++++++-- .../selector/move/generic/ChangeMoveTest.java | 4 ++- .../move/generic/PillarChangeMoveTest.java | 4 ++- .../move/generic/PillarSwapMoveTest.java | 4 ++- .../selector/move/generic/SwapMoveTest.java | 4 ++- .../ScoreDirectorFactoryFactoryTest.java | 7 +++-- .../easy/EasyScoreDirectorSemanticsTest.java | 4 ++- .../director/easy/EasyScoreDirectorTest.java | 2 +- 13 files changed, 59 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/api/score/analysis/ConstraintAnalysis.java b/core/src/main/java/ai/timefold/solver/core/api/score/analysis/ConstraintAnalysis.java index 0cced4e003..9a717545bf 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/score/analysis/ConstraintAnalysis.java +++ b/core/src/main/java/ai/timefold/solver/core/api/score/analysis/ConstraintAnalysis.java @@ -31,8 +31,9 @@ * non-empty if constraint has matches. * This is a {@link List} to simplify access to individual elements, * but it contains no duplicates just like {@link HashSet} wouldn't. - * @param matchCount -1 if {@link #matches()} is null, 0 if {@link #matches()} is empty, - * and {@link #matches() matches() size} otherwise. + * @param matchCount -1 if analysis not available, + * 0 if constraint has no matches, + * positive if constraint has matches. */ public record ConstraintAnalysis>(@NonNull ConstraintRef constraintRef, @NonNull Score_ weight, @NonNull Score_ score, @Nullable List> matches, int matchCount) { diff --git a/core/src/main/java/ai/timefold/solver/core/api/score/stream/ConstraintJustification.java b/core/src/main/java/ai/timefold/solver/core/api/score/stream/ConstraintJustification.java index 6e74858715..9bb5e8b68f 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/score/stream/ConstraintJustification.java +++ b/core/src/main/java/ai/timefold/solver/core/api/score/stream/ConstraintJustification.java @@ -14,7 +14,7 @@ * All classes used as constraint justifications must implement this interface. * *

- * Implementing classes ("implementations") may decide to implement {@link Comparable} + * Implementations may decide to implement {@link Comparable} * to preserve order of instances when displayed in user interfaces, logs etc. * This is entirely optional. * diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/constraint/ConstraintMatchPolicy.java b/core/src/main/java/ai/timefold/solver/core/impl/score/constraint/ConstraintMatchPolicy.java index 3cc03cf7e0..bac2a9ed77 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/constraint/ConstraintMatchPolicy.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/constraint/ConstraintMatchPolicy.java @@ -2,12 +2,31 @@ import ai.timefold.solver.core.api.solver.ScoreAnalysisFetchPolicy; +import org.jspecify.annotations.NullMarked; + +/** + * Determines whether constraint match is enabled and whether constraint match justification is enabled. + * + * @see ai.timefold.solver.core.api.score.constraint.ConstraintMatch + * @see ai.timefold.solver.core.api.score.stream.ConstraintJustification + */ +@NullMarked public enum ConstraintMatchPolicy { DISABLED(false, false), ENABLED_WITHOUT_JUSTIFICATIONS(true, false), ENABLED(true, true); + /** + * To achieve the most performance out of the underlying solver, + * the policy should match whatever policy was used for score analysis. + * For example, if the fetch policy specifies that only match counts are necessary and not matches themselves + * ({@link ScoreAnalysisFetchPolicy#FETCH_MATCH_COUNT}), + * we can configure the solver to not produce justifications ({@link #ENABLED_WITHOUT_JUSTIFICATIONS}). + * + * @param scoreAnalysisFetchPolicy + * @return Match policy best suited for the given fetch policy. + */ public static ConstraintMatchPolicy match(ScoreAnalysisFetchPolicy scoreAnalysisFetchPolicy) { return switch (scoreAnalysisFetchPolicy) { case FETCH_MATCH_COUNT, FETCH_SHALLOW -> ENABLED_WITHOUT_JUSTIFICATIONS; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirectorFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirectorFactory.java index 421407a443..08bdcff052 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirectorFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirectorFactory.java @@ -93,11 +93,6 @@ public void setTrackingWorkingSolution(boolean trackingWorkingSolution) { // Complex methods // ************************************************************************ - @Override - public InnerScoreDirector buildScoreDirector() { - return buildScoreDirector(false, ConstraintMatchPolicy.DISABLED); - } - @Override public void assertScoreFromScratch(Solution_ solution) { // Get the score before uncorruptedScoreDirector.calculateScore() modifies it diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirectorFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirectorFactory.java index 2dcd945a4d..e73659839f 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirectorFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirectorFactory.java @@ -27,37 +27,12 @@ public interface InnerScoreDirectorFactory getScoreDefinition(); @Override - InnerScoreDirector buildScoreDirector(); - - /** - * Like {@link #buildScoreDirector()}, but optionally enables {@link ConstraintMatch} tracking and look up - * where possible and necessary. - * - * @param lookUpEnabled true if a {@link ScoreDirector} implementation should track all working objects - * for {@link ScoreDirector#lookUpWorkingObject(Object)} - * @param constraintMatchPolicy how should the {@link ScoreDirector} track {@link ConstraintMatch}es - * @return never null - * @see InnerScoreDirector#getConstraintMatchPolicy() - * @see InnerScoreDirector#getConstraintMatchTotalMap() - */ default InnerScoreDirector buildScoreDirector(boolean lookUpEnabled, ConstraintMatchPolicy constraintMatchPolicy) { return buildScoreDirector(lookUpEnabled, constraintMatchPolicy, true); } - /** - * Like {@link #buildScoreDirector()}, but optionally enables {@link ConstraintMatch} tracking and look up - * where possible and necessary. - * - * @param lookUpEnabled true if a {@link ScoreDirector} implementation should track all working objects - * for {@link ScoreDirector#lookUpWorkingObject(Object)} - * @param constraintMatchPolicy how should the {@link ScoreDirector} implementation do {@link ConstraintMatch}, if at all. - * @param expectShadowVariablesInCorrectState true, unless you have an exceptional reason. - * See {@link InnerScoreDirector#expectShadowVariablesInCorrectState()} for details. - * @return never null - * @see InnerScoreDirector#getConstraintMatchPolicy() - * @see InnerScoreDirector#getConstraintMatchTotalMap() - */ + @Override InnerScoreDirector buildScoreDirector(boolean lookUpEnabled, ConstraintMatchPolicy constraintMatchPolicy, boolean expectShadowVariablesInCorrectState); @@ -76,7 +51,7 @@ InnerScoreDirector buildScoreDirector(boolean lookUpEnabled, default InnerScoreDirector buildDerivedScoreDirector(boolean lookUpEnabled, ConstraintMatchPolicy constraintMatchPolicy) { // Most score directors don't need derived status; CS will override this. - return buildScoreDirector(lookUpEnabled, constraintMatchPolicy, true); + return buildScoreDirector(lookUpEnabled, constraintMatchPolicy); } /** diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactory.java index 316970891b..d699253c8b 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/ScoreDirectorFactory.java @@ -1,7 +1,9 @@ package ai.timefold.solver.core.impl.score.director; import ai.timefold.solver.core.api.domain.solution.PlanningSolution; +import ai.timefold.solver.core.api.score.constraint.ConstraintMatch; import ai.timefold.solver.core.api.score.director.ScoreDirector; +import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; /** * Builds a {@link ScoreDirector}. @@ -11,10 +13,19 @@ public interface ScoreDirectorFactory { /** - * Creates a new {@link ScoreDirector} instance. + * Like {@link #buildScoreDirector(boolean, ConstraintMatchPolicy, boolean)}, + * with the final parameter set to true. * + * @param lookUpEnabled true if a {@link ScoreDirector} implementation should track all working objects + * for {@link ScoreDirector#lookUpWorkingObject(Object)} + * @param constraintMatchPolicy how should the {@link ScoreDirector} track {@link ConstraintMatch constraint matches}. * @return never null */ - ScoreDirector buildScoreDirector(); + default ScoreDirector buildScoreDirector(boolean lookUpEnabled, ConstraintMatchPolicy constraintMatchPolicy) { + return buildScoreDirector(lookUpEnabled, constraintMatchPolicy, true); + } + + ScoreDirector buildScoreDirector(boolean lookUpEnabled, ConstraintMatchPolicy constraintMatchPolicy, + boolean expectShadowVariablesInCorrectState); } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/ChangeMoveTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/ChangeMoveTest.java index 8b8fefc6ec..cddb9d0751 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/ChangeMoveTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/ChangeMoveTest.java @@ -10,6 +10,7 @@ import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore; import ai.timefold.solver.core.api.score.director.ScoreDirector; import ai.timefold.solver.core.impl.domain.variable.descriptor.GenuineVariableDescriptor; +import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; import ai.timefold.solver.core.impl.score.director.ScoreDirectorFactory; import ai.timefold.solver.core.impl.score.director.easy.EasyScoreDirectorFactory; import ai.timefold.solver.core.impl.testdata.domain.TestdataEntity; @@ -60,7 +61,8 @@ void doMove() { ScoreDirectorFactory scoreDirectorFactory = new EasyScoreDirectorFactory<>(TestdataEntityProvidingSolution.buildSolutionDescriptor(), solution -> SimpleScore.ZERO); - ScoreDirector scoreDirector = scoreDirectorFactory.buildScoreDirector(); + ScoreDirector scoreDirector = + scoreDirectorFactory.buildScoreDirector(false, ConstraintMatchPolicy.DISABLED); GenuineVariableDescriptor variableDescriptor = TestdataEntityProvidingEntity.buildVariableDescriptorForValue(); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/PillarChangeMoveTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/PillarChangeMoveTest.java index 16f2cc27c7..12acf7f61a 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/PillarChangeMoveTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/PillarChangeMoveTest.java @@ -12,6 +12,7 @@ import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore; import ai.timefold.solver.core.api.score.director.ScoreDirector; import ai.timefold.solver.core.impl.domain.variable.descriptor.GenuineVariableDescriptor; +import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; import ai.timefold.solver.core.impl.score.director.ScoreDirectorFactory; import ai.timefold.solver.core.impl.score.director.easy.EasyScoreDirectorFactory; import ai.timefold.solver.core.impl.testdata.domain.TestdataEntity; @@ -75,7 +76,8 @@ void doMove() { ScoreDirectorFactory scoreDirectorFactory = new EasyScoreDirectorFactory<>(TestdataEntityProvidingSolution.buildSolutionDescriptor(), solution -> SimpleScore.ZERO); - ScoreDirector scoreDirector = scoreDirectorFactory.buildScoreDirector(); + ScoreDirector scoreDirector = + scoreDirectorFactory.buildScoreDirector(false, ConstraintMatchPolicy.DISABLED); GenuineVariableDescriptor variableDescriptor = TestdataEntityProvidingEntity .buildVariableDescriptorForValue(); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/PillarSwapMoveTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/PillarSwapMoveTest.java index 4d8cf0fa02..9f55af1523 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/PillarSwapMoveTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/PillarSwapMoveTest.java @@ -12,6 +12,7 @@ import ai.timefold.solver.core.api.score.director.ScoreDirector; import ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.GenuineVariableDescriptor; +import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; import ai.timefold.solver.core.impl.score.director.ScoreDirectorFactory; import ai.timefold.solver.core.impl.score.director.easy.EasyScoreDirectorFactory; import ai.timefold.solver.core.impl.testdata.domain.TestdataEntity; @@ -128,7 +129,8 @@ void doMove() { ScoreDirectorFactory scoreDirectorFactory = new EasyScoreDirectorFactory<>(TestdataEntityProvidingSolution.buildSolutionDescriptor(), solution -> SimpleScore.ZERO); - ScoreDirector scoreDirector = scoreDirectorFactory.buildScoreDirector(); + ScoreDirector scoreDirector = + scoreDirectorFactory.buildScoreDirector(false, ConstraintMatchPolicy.DISABLED); List> variableDescriptorList = TestdataEntityProvidingEntity .buildEntityDescriptor().getGenuineVariableDescriptorList(); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/SwapMoveTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/SwapMoveTest.java index 670fb67f35..da06267e49 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/SwapMoveTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/SwapMoveTest.java @@ -12,6 +12,7 @@ import ai.timefold.solver.core.api.score.director.ScoreDirector; import ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.GenuineVariableDescriptor; +import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; import ai.timefold.solver.core.impl.score.director.ScoreDirectorFactory; import ai.timefold.solver.core.impl.score.director.easy.EasyScoreDirectorFactory; import ai.timefold.solver.core.impl.testdata.domain.TestdataEntity; @@ -103,7 +104,8 @@ void doMove() { ScoreDirectorFactory scoreDirectorFactory = new EasyScoreDirectorFactory<>(TestdataEntityProvidingSolution.buildSolutionDescriptor(), solution -> SimpleScore.ZERO); - ScoreDirector scoreDirector = scoreDirectorFactory.buildScoreDirector(); + ScoreDirector scoreDirector = + scoreDirectorFactory.buildScoreDirector(false, ConstraintMatchPolicy.DISABLED); EntityDescriptor entityDescriptor = TestdataEntityProvidingEntity .buildEntityDescriptor(); 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 761a060cc5..30bb103328 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 @@ -15,6 +15,7 @@ import ai.timefold.solver.core.api.score.stream.ConstraintStreamImplType; import ai.timefold.solver.core.config.score.director.ScoreDirectorFactoryConfig; import ai.timefold.solver.core.config.solver.EnvironmentMode; +import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; import ai.timefold.solver.core.impl.score.director.incremental.IncrementalScoreDirector; import ai.timefold.solver.core.impl.score.director.stream.BavetConstraintStreamScoreDirectorFactory; import ai.timefold.solver.core.impl.testdata.domain.TestdataSolution; @@ -36,7 +37,8 @@ void incrementalScoreCalculatorWithCustomProperties() { ScoreDirectorFactory scoreDirectorFactory = buildTestdataScoreDirectoryFactory(config); IncrementalScoreDirector scoreDirector = - (IncrementalScoreDirector) scoreDirectorFactory.buildScoreDirector(); + (IncrementalScoreDirector) scoreDirectorFactory.buildScoreDirector(false, + ConstraintMatchPolicy.DISABLED); TestCustomPropertiesIncrementalScoreCalculator scoreCalculator = (TestCustomPropertiesIncrementalScoreCalculator) scoreDirector .getIncrementalScoreCalculator(); @@ -59,7 +61,8 @@ void buildWithAssertionScoreDirectorFactory() { ScoreDirectorFactory assertionScoreDirectorFactory = scoreDirectorFactory.getAssertionScoreDirectorFactory(); IncrementalScoreDirector assertionScoreDirector = - (IncrementalScoreDirector) assertionScoreDirectorFactory.buildScoreDirector(); + (IncrementalScoreDirector) assertionScoreDirectorFactory.buildScoreDirector(false, + ConstraintMatchPolicy.DISABLED); IncrementalScoreCalculator assertionScoreCalculator = assertionScoreDirector.getIncrementalScoreCalculator(); 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 658e42c85a..8391916772 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 @@ -10,6 +10,7 @@ import ai.timefold.solver.core.config.score.director.ScoreDirectorFactoryConfig; import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; +import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; import ai.timefold.solver.core.impl.score.director.AbstractScoreDirectorSemanticsTest; import ai.timefold.solver.core.impl.score.director.InnerScoreDirectorFactory; import ai.timefold.solver.core.impl.score.director.ScoreDirectorFactory; @@ -70,7 +71,8 @@ void easyScoreCalculatorWithCustomProperties() { config.setEasyScoreCalculatorCustomProperties(customProperties); EasyScoreDirector scoreDirector = - (EasyScoreDirector) buildTestdataScoreDirectoryFactory(config).buildScoreDirector(); + (EasyScoreDirector) buildTestdataScoreDirectoryFactory(config) + .buildScoreDirector(false, ConstraintMatchPolicy.DISABLED); TestCustomPropertiesEasyScoreCalculator scoreCalculator = (TestCustomPropertiesEasyScoreCalculator) scoreDirector .getEasyScoreCalculator(); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirectorTest.java index ec7f874cc2..ca24dc5d89 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirectorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirectorTest.java @@ -22,7 +22,7 @@ void shadowVariableCorruption() { (solution_) -> SimpleScore.of(0)); scoreDirectorFactory .setInitializingScoreTrend(InitializingScoreTrend.buildUniformTrend(InitializingScoreTrendLevel.ONLY_DOWN, 1)); - try (var scoreDirector = scoreDirectorFactory.buildScoreDirector(false, ConstraintMatchPolicy.DISABLED, true)) { + try (var scoreDirector = scoreDirectorFactory.buildScoreDirector(false, ConstraintMatchPolicy.DISABLED)) { var solution = new TestdataCorruptedShadowedSolution("s1"); var v1 = new TestdataValue("v1"); var v2 = new TestdataValue("v2");