Skip to content

Commit 7973b63

Browse files
Christopher-ChianelliChristopher-Chianelli
committed
chore: handle inconsistent solutions in update, analyze and recommendations
1 parent d580623 commit 7973b63

3 files changed

Lines changed: 76 additions & 7 deletions

File tree

‎core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,10 @@ public void updateShadowVariables() {
489489
lastVariableUpdateWasSuccessful = shadowVariableSupport.updateShadowVariables();
490490
}
491491

492+
public boolean isLastVariableUpdateWasSuccessful() {
493+
return lastVariableUpdateWasSuccessful;
494+
}
495+
492496
/**
493497
* This function clears all listener events that have been generated without triggering any of them.
494498
* Using this method requires caution because clearing the event queue can lead to inconsistent states.

‎core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,23 @@ public ScoreDirectorFactory<Solution_, Score_> getScoreDirectorFactory() {
5151
@Override
5252
public Score_ update(Solution_ solution, SolutionUpdatePolicy solutionUpdatePolicy) {
5353
if (solutionUpdatePolicy == SolutionUpdatePolicy.NO_UPDATE) {
54-
throw new IllegalArgumentException("Can not call " + this.getClass().getSimpleName()
55-
+ ".update() with this solutionUpdatePolicy (" + solutionUpdatePolicy + ").");
54+
throw new IllegalArgumentException(
55+
"Can not call %s.update() with this solutionUpdatePolicy (%s)."
56+
.formatted(this.getClass().getSimpleName(), solutionUpdatePolicy));
5657
}
57-
return callScoreDirector(solution, solutionUpdatePolicy,
58+
return callScoreDirector("Solution update", solution, solutionUpdatePolicy,
5859
s -> s.getSolutionDescriptor().getScore(s.getWorkingSolution()), ConstraintMatchPolicy.DISABLED, false);
5960
}
6061

61-
private <Result_> Result_ callScoreDirector(Solution_ solution, SolutionUpdatePolicy solutionUpdatePolicy,
62+
private <Result_> Result_ callScoreDirector(String feature, Solution_ solution, SolutionUpdatePolicy solutionUpdatePolicy,
6263
Function<InnerScoreDirector<Solution_, Score_>, Result_> function, ConstraintMatchPolicy constraintMatchPolicy,
6364
boolean cloneSolution) {
6465
var isShadowVariableUpdateEnabled = solutionUpdatePolicy.isShadowVariableUpdateEnabled();
6566
var nonNullSolution = Objects.requireNonNull(solution);
67+
var allowsInconsistent = getScoreDirectorFactory().getSolutionDescriptor().hasAnyShadowVariablesInconsistentMember();
6668
try (var scoreDirector = getScoreDirectorFactory().createScoreDirectorBuilder().withLookUpEnabled(cloneSolution)
6769
.withConstraintMatchPolicy(constraintMatchPolicy)
70+
.withIgnoreInconsistentSolutions(!allowsInconsistent)
6871
.withExpectShadowVariablesInCorrectState(!isShadowVariableUpdateEnabled).build()) {
6972
nonNullSolution = cloneSolution ? scoreDirector.cloneSolution(nonNullSolution) : nonNullSolution;
7073
if (isShadowVariableUpdateEnabled) {
@@ -82,7 +85,16 @@ private <Result_> Result_ callScoreDirector(Solution_ solution, SolutionUpdatePo
8285
Maybe use Constraint Streams instead of Easy or Incremental score calculator?""");
8386
}
8487
if (solutionUpdatePolicy.isScoreUpdateEnabled()) {
85-
scoreDirector.calculateScore();
88+
var score = scoreDirector.calculateScore();
89+
if (score.isInvalid()) {
90+
throw new IllegalArgumentException("""
91+
The solution (%s) is inconsistent. %s requires a consistent solution to be passed.
92+
""".formatted(solution, feature));
93+
}
94+
} else if (!scoreDirector.isLastVariableUpdateWasSuccessful()) {
95+
throw new IllegalArgumentException("""
96+
The solution (%s) is inconsistent. %s requires a consistent solution to be passed.
97+
""".formatted(solution, feature));
8698
}
8799
return function.apply(scoreDirector);
88100
}
@@ -112,7 +124,7 @@ public ScoreAnalysis<Score_> analyze(Solution_ solution, ScoreAnalysisFetchPolic
112124
var enterpriseService =
113125
TimefoldSolverEnterpriseService.loadOrFail(TimefoldSolverEnterpriseService.Feature.SCORE_ANALYSIS);
114126
var currentScore = (Score_) scoreDirectorFactory.getSolutionDescriptor().getScore(solution);
115-
var analysis = callScoreDirector(solution, solutionUpdatePolicy,
127+
var analysis = callScoreDirector("Solution analysis", solution, solutionUpdatePolicy,
116128
scoreDirector -> enterpriseService.analyze(scoreDirector.calculateScore(),
117129
scoreDirector.getConstraintMatchTotalMap(), fetchPolicy),
118130
ConstraintMatchPolicy.match(fetchPolicy), false);
@@ -134,7 +146,7 @@ public <In_, Out_> List<RecommendedAssignment<Out_, Score_>> recommendAssignment
134146
ScoreAnalysisFetchPolicy fetchPolicy) {
135147
var enterpriseService =
136148
TimefoldSolverEnterpriseService.loadOrFail(TimefoldSolverEnterpriseService.Feature.RECOMMENDATIONS);
137-
return callScoreDirector(
149+
return callScoreDirector("Recommended assignment",
138150
solution, SolutionUpdatePolicy.UPDATE_ALL, enterpriseService.buildRecommender(solverFactory, solution,
139151
evaluatedEntityOrElement, propositionFunction, fetchPolicy),
140152
ConstraintMatchPolicy.match(fetchPolicy), true);

‎core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static org.assertj.core.api.Assertions.assertThatCode;
55
import static org.assertj.core.api.SoftAssertions.assertSoftly;
66

7+
import java.time.Duration;
78
import java.util.Arrays;
89
import java.util.List;
910
import java.util.function.Function;
@@ -23,6 +24,10 @@
2324
import ai.timefold.solver.core.testdomain.shadow.concurrent.TestdataConcurrentEntity;
2425
import ai.timefold.solver.core.testdomain.shadow.concurrent.TestdataConcurrentSolution;
2526
import ai.timefold.solver.core.testdomain.shadow.concurrent.TestdataConcurrentValue;
27+
import ai.timefold.solver.core.testdomain.shadow.no_inconsistent_field.TestdataDependencyNoInconsistentFieldConstraintProvider;
28+
import ai.timefold.solver.core.testdomain.shadow.no_inconsistent_field.TestdataDependencyNoInconsistentFieldEntity;
29+
import ai.timefold.solver.core.testdomain.shadow.no_inconsistent_field.TestdataDependencyNoInconsistentFieldSolution;
30+
import ai.timefold.solver.core.testdomain.shadow.no_inconsistent_field.TestdataDependencyNoInconsistentFieldValue;
2631

2732
import org.assertj.core.api.SoftAssertions;
2833
import org.junit.jupiter.api.Test;
@@ -113,6 +118,30 @@ void updateEverythingList(SolutionManagerSource solutionManagerSource) {
113118
});
114119
}
115120

121+
@ParameterizedTest
122+
@EnumSource(SolutionManagerSource.class)
123+
void updateInconsistent(SolutionManagerSource solutionManagerSource) {
124+
var entity1 = new TestdataDependencyNoInconsistentFieldEntity("e1");
125+
var valueA1 = new TestdataDependencyNoInconsistentFieldValue("a1");
126+
var valueA2 = new TestdataDependencyNoInconsistentFieldValue("a2", Duration.ofHours(1L), List.of(valueA1));
127+
entity1.setValues(List.of(valueA2, valueA1));
128+
var inconsistentSolution =
129+
new TestdataDependencyNoInconsistentFieldSolution(List.of(entity1), List.of(valueA1, valueA2));
130+
131+
var solverFactory = SolverFactory.create(new SolverConfig()
132+
.withSolutionClass(TestdataDependencyNoInconsistentFieldSolution.class)
133+
.withEntityClasses(TestdataDependencyNoInconsistentFieldEntity.class,
134+
TestdataDependencyNoInconsistentFieldValue.class)
135+
.withConstraintProviderClass(TestdataDependencyNoInconsistentFieldConstraintProvider.class));
136+
var solutionManager = solutionManagerSource.createSolutionManager(solverFactory);
137+
assertThat(solutionManager).isNotNull();
138+
139+
assertThatCode(() -> solutionManager.update(inconsistentSolution))
140+
.isInstanceOf(IllegalArgumentException.class)
141+
.hasMessageContainingAll("The solution (",
142+
"is inconsistent", "Solution update", "requires a consistent solution");
143+
}
144+
116145
private void assertShadowedListValueAllNull(SoftAssertions softly, TestdataListValueWithShadowHistory current) {
117146
softly.assertThat(current.getIndex()).isNull();
118147
softly.assertThat(current.getEntity()).isNull();
@@ -149,6 +178,30 @@ void updateOnlyShadowVariables(SolutionManagerSource solutionManagerSource) {
149178
});
150179
}
151180

181+
@ParameterizedTest
182+
@EnumSource(SolutionManagerSource.class)
183+
void updateOnlyShadowVariablesInconsistent(SolutionManagerSource solutionManagerSource) {
184+
var entity1 = new TestdataDependencyNoInconsistentFieldEntity("e1");
185+
var valueA1 = new TestdataDependencyNoInconsistentFieldValue("a1");
186+
var valueA2 = new TestdataDependencyNoInconsistentFieldValue("a2", Duration.ofHours(1L), List.of(valueA1));
187+
entity1.setValues(List.of(valueA2, valueA1));
188+
var inconsistentSolution =
189+
new TestdataDependencyNoInconsistentFieldSolution(List.of(entity1), List.of(valueA1, valueA2));
190+
191+
var solverFactory = SolverFactory.create(new SolverConfig()
192+
.withSolutionClass(TestdataDependencyNoInconsistentFieldSolution.class)
193+
.withEntityClasses(TestdataDependencyNoInconsistentFieldEntity.class,
194+
TestdataDependencyNoInconsistentFieldValue.class)
195+
.withConstraintProviderClass(TestdataDependencyNoInconsistentFieldConstraintProvider.class));
196+
var solutionManager = solutionManagerSource.createSolutionManager(solverFactory);
197+
assertThat(solutionManager).isNotNull();
198+
199+
assertThatCode(() -> solutionManager.update(inconsistentSolution, SolutionUpdatePolicy.UPDATE_SHADOW_VARIABLES_ONLY))
200+
.isInstanceOf(IllegalArgumentException.class)
201+
.hasMessageContainingAll("The solution (",
202+
"is inconsistent", "Solution update", "requires a consistent solution");
203+
}
204+
152205
@ParameterizedTest
153206
@EnumSource(SolutionManagerSource.class)
154207
void updateOnlyScore(SolutionManagerSource solutionManagerSource) {

0 commit comments

Comments
 (0)