Skip to content

Commit 9d6cc3e

Browse files
authored
feat: allow ConstraintVerifier to update all shadow variables
1 parent 9574cab commit 9d6cc3e

File tree

28 files changed

+1658
-898
lines changed

28 files changed

+1658
-898
lines changed

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/AbstractNotifiable.java

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ static <Solution_> EntityNotifiable<Solution_> buildNotifiable(
3434
((ListVariableListener<Solution_, Object, Object>) variableListener),
3535
new ArrayDeque<>(), globalOrder);
3636
} else {
37-
VariableListener<Solution_, Object> basicVariableListener = (VariableListener<Solution_, Object>) variableListener;
37+
var basicVariableListener = (VariableListener<Solution_, Object>) variableListener;
3838
return new VariableListenerNotifiable<>(
3939
scoreDirector,
4040
basicVariableListener,
@@ -80,26 +80,31 @@ public void closeVariableListener() {
8080
variableListener.close();
8181
}
8282

83+
@Override
84+
public void clearAllNotifications() {
85+
notificationQueue.clear();
86+
}
87+
8388
@Override
8489
public void triggerAllNotifications() {
85-
int notifiedCount = 0;
86-
for (Notification<Solution_, ? super T> notification : notificationQueue) {
90+
var notifiedCount = 0;
91+
for (var notification : notificationQueue) {
8792
notification.triggerAfter(variableListener, scoreDirector);
8893
notifiedCount++;
8994
}
9095
if (notifiedCount != notificationQueue.size()) {
91-
throw new IllegalStateException("The variableListener (" + variableListener.getClass()
92-
+ ") has been notified with notifiedCount (" + notifiedCount
93-
+ ") but after being triggered, its notificationCount (" + notificationQueue.size()
94-
+ ") is different.\n"
95-
+ "Maybe that variableListener (" + variableListener.getClass()
96-
+ ") changed an upstream shadow variable (which is illegal).");
96+
throw new IllegalStateException(
97+
"""
98+
The variableListener (%s) has been notified with notifiedCount (%d) but after being triggered, its notificationCount (%d) is different.
99+
Maybe that variableListener (%s) changed an upstream shadow variable (which is illegal)."""
100+
.formatted(variableListener.getClass(), notifiedCount, notificationQueue.size(),
101+
variableListener.getClass()));
97102
}
98103
notificationQueue.clear();
99104
}
100105

101106
@Override
102107
public String toString() {
103-
return "(" + globalOrder + ") " + variableListener;
108+
return "(%d) %s".formatted(globalOrder, variableListener);
104109
}
105110
}

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/Notifiable.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ public interface Notifiable {
1818
*/
1919
void resetWorkingSolution();
2020

21+
/**
22+
* Clear all notifications without triggering any related event logic.
23+
*/
24+
void clearAllNotifications();
25+
2126
/**
2227
* Trigger all queued notifications.
2328
*/

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,15 @@ public void forceTriggerAllVariableListeners(Solution_ workingSolution) {
368368
triggerVariableListenersInNotificationQueues();
369369
}
370370

371+
/**
372+
* Clear all variable listeners without triggering any logic.
373+
* The goal is to clear all queues and avoid executing custom listener logic.
374+
*/
375+
public void clearAllVariableListenerEvents() {
376+
notifiableRegistry.getAll().forEach(Notifiable::clearAllNotifications);
377+
notificationQueuesAreEmpty = true;
378+
}
379+
371380
private void simulateGenuineVariableChange(Object entity) {
372381
var entityDescriptor = scoreDirector.getSolutionDescriptor()
373382
.findEntityDescriptorOrFail(entity.getClass());
@@ -389,11 +398,12 @@ private void simulateGenuineVariableChange(Object entity) {
389398

390399
public void assertNotificationQueuesAreEmpty() {
391400
if (!notificationQueuesAreEmpty) {
392-
throw new IllegalStateException("The notificationQueues might not be empty (" + notificationQueuesAreEmpty
393-
+ ") so any shadow variables might be stale so score calculation is unreliable.\n"
394-
+ "Maybe a " + ScoreDirector.class.getSimpleName() + ".before*() method was called"
395-
+ " without calling " + ScoreDirector.class.getSimpleName() + ".triggerVariableListeners(),"
396-
+ " before calling " + ScoreDirector.class.getSimpleName() + ".calculateScore().");
401+
throw new IllegalStateException(
402+
"""
403+
The notificationQueues might not be empty (%s) so any shadow variables might be stale so score calculation is unreliable.
404+
Maybe a %s.before*() method was called without calling %s.triggerVariableListeners(), before calling %s.calculateScore()."""
405+
.formatted(notificationQueuesAreEmpty, ScoreDirector.class.getSimpleName(),
406+
ScoreDirector.class.getSimpleName(), ScoreDirector.class.getSimpleName()));
397407
}
398408
}
399409

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

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import java.util.IdentityHashMap;
77
import java.util.LinkedHashSet;
88
import java.util.List;
9-
import java.util.Map;
109
import java.util.Objects;
1110
import java.util.Set;
1211
import java.util.function.Consumer;
@@ -24,7 +23,6 @@
2423
import ai.timefold.solver.core.config.solver.EnvironmentMode;
2524
import ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptor;
2625
import ai.timefold.solver.core.impl.domain.lookup.LookUpManager;
27-
import ai.timefold.solver.core.impl.domain.solution.ConstraintWeightSupplier;
2826
import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor;
2927
import ai.timefold.solver.core.impl.domain.variable.ListVariableStateSupply;
3028
import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor;
@@ -312,17 +310,17 @@ protected void setWorkingEntityListDirty() {
312310
@Override
313311
public Solution_ cloneSolution(Solution_ originalSolution) {
314312
SolutionDescriptor<Solution_> solutionDescriptor = getSolutionDescriptor();
315-
Score_ originalScore = solutionDescriptor.getScore(originalSolution);
316-
Solution_ cloneSolution = solutionDescriptor.getSolutionCloner().cloneSolution(originalSolution);
317-
Score_ cloneScore = solutionDescriptor.getScore(cloneSolution);
313+
var originalScore = solutionDescriptor.getScore(originalSolution);
314+
var cloneSolution = solutionDescriptor.getSolutionCloner().cloneSolution(originalSolution);
315+
var cloneScore = solutionDescriptor.getScore(cloneSolution);
318316
if (scoreDirectorFactory.isAssertClonedSolution()) {
319317
if (!Objects.equals(originalScore, cloneScore)) {
320318
throw new CloningCorruptionException("""
321319
Cloning corruption: the original's score (%s) is different from the clone's score (%s).
322320
Check the %s."""
323321
.formatted(originalScore, cloneScore, SolutionCloner.class.getSimpleName()));
324322
}
325-
Map<Object, Object> originalEntityMap = new IdentityHashMap<>();
323+
var originalEntityMap = new IdentityHashMap<>();
326324
solutionDescriptor.visitAllEntities(originalSolution,
327325
originalEntity -> originalEntityMap.put(originalEntity, null));
328326
solutionDescriptor.visitAllEntities(cloneSolution, cloneEntity -> {
@@ -343,6 +341,16 @@ public void triggerVariableListeners() {
343341
variableListenerSupport.triggerVariableListenersInNotificationQueues();
344342
}
345343

344+
/**
345+
* This function clears all listener events that have been generated without triggering any of them.
346+
* Using this method requires caution because clearing the event queue can lead to inconsistent states.
347+
* This occurs when the shadow variables are not updated,
348+
* causing constraints reliant on these variables to be inaccurately evaluated.
349+
*/
350+
protected void clearVariableListenerEvents() {
351+
variableListenerSupport.clearAllVariableListenerEvents();
352+
}
353+
346354
@Override
347355
public void forceTriggerVariableListeners() {
348356
variableListenerSupport.forceTriggerAllVariableListeners(getWorkingSolution());
@@ -560,9 +568,9 @@ public void afterProblemPropertyChanged(Object problemFactOrEntity) {
560568
@Override
561569
public void beforeProblemFactRemoved(Object problemFact) {
562570
if (isConstraintConfiguration(problemFact)) {
563-
throw new IllegalStateException("Attempted to remove constraint configuration (" + problemFact +
564-
") from solution (" + workingSolution + ").\n" +
565-
"Maybe use before/afterProblemPropertyChanged(...) instead.");
571+
throw new IllegalStateException("""
572+
Attempted to remove constraint configuration (%s) from solution (%s).
573+
Maybe use before/afterProblemPropertyChanged(...) instead.""".formatted(problemFact, workingSolution));
566574
}
567575
}
568576

@@ -580,17 +588,19 @@ public void afterProblemFactRemoved(Object problemFact) {
580588
@Override
581589
public <E> @Nullable E lookUpWorkingObject(@Nullable E externalObject) {
582590
if (!lookUpEnabled) {
583-
throw new IllegalStateException("When lookUpEnabled (" + lookUpEnabled
584-
+ ") is disabled in the constructor, this method should not be called.");
591+
throw new IllegalStateException(
592+
"When lookUpEnabled (%s) is disabled in the constructor, this method should not be called."
593+
.formatted(lookUpEnabled));
585594
}
586595
return lookUpManager.lookUpWorkingObject(externalObject);
587596
}
588597

589598
@Override
590599
public <E> @Nullable E lookUpWorkingObjectOrReturnNull(@Nullable E externalObject) {
591600
if (!lookUpEnabled) {
592-
throw new IllegalStateException("When lookUpEnabled (" + lookUpEnabled
593-
+ ") is disabled in the constructor, this method should not be called.");
601+
throw new IllegalStateException(
602+
"When lookUpEnabled (%s) is disabled in the constructor, this method should not be called."
603+
.formatted(lookUpEnabled));
594604
}
595605
return lookUpManager.lookUpWorkingObjectOrReturnNull(externalObject);
596606
}
@@ -601,7 +611,7 @@ public void afterProblemFactRemoved(Object problemFact) {
601611

602612
@Override
603613
public void assertExpectedWorkingScore(Score_ expectedWorkingScore, Object completedAction) {
604-
Score_ workingScore = calculateScore();
614+
var workingScore = calculateScore();
605615
if (!expectedWorkingScore.equals(workingScore)) {
606616
throw new ScoreCorruptionException("""
607617
Score corruption (%s): the expectedWorkingScore (%s) is not the workingScore (%s) \
@@ -613,15 +623,15 @@ after completedAction (%s)."""
613623

614624
@Override
615625
public void assertShadowVariablesAreNotStale(Score_ expectedWorkingScore, Object completedAction) {
616-
String violationMessage = variableListenerSupport.createShadowVariablesViolationMessage();
626+
var violationMessage = variableListenerSupport.createShadowVariablesViolationMessage();
617627
if (violationMessage != null) {
618628
throw new VariableCorruptionException("""
619629
%s corruption after completedAction (%s):
620630
%s"""
621631
.formatted(VariableListener.class.getSimpleName(), completedAction, violationMessage));
622632
}
623633

624-
Score_ workingScore = calculateScore();
634+
var workingScore = calculateScore();
625635
if (!expectedWorkingScore.equals(workingScore)) {
626636
assertWorkingScoreFromScratch(workingScore,
627637
"assertShadowVariablesAreNotStale(" + expectedWorkingScore + ", " + completedAction + ")");
@@ -642,8 +652,8 @@ public void assertShadowVariablesAreNotStale(Score_ expectedWorkingScore, Object
642652
* @return never null
643653
*/
644654
protected String buildShadowVariableAnalysis(boolean predicted) {
645-
String violationMessage = variableListenerSupport.createShadowVariablesViolationMessage();
646-
String workingLabel = predicted ? "working" : "corrupted";
655+
var violationMessage = variableListenerSupport.createShadowVariablesViolationMessage();
656+
var workingLabel = predicted ? "working" : "corrupted";
647657
if (violationMessage == null) {
648658
return """
649659
Shadow variable corruption in the %s scoreDirector:
@@ -894,8 +904,8 @@ private void iterateAndAddIfFound(List<MatchAnalysis<Score_>> referenceList, Lis
894904
}
895905

896906
protected boolean isConstraintConfiguration(Object problemFactOrEntity) {
897-
SolutionDescriptor<Solution_> solutionDescriptor = scoreDirectorFactory.getSolutionDescriptor();
898-
ConstraintWeightSupplier<Solution_, Score_> constraintWeightSupplier = solutionDescriptor.getConstraintWeightSupplier();
907+
var solutionDescriptor = scoreDirectorFactory.getSolutionDescriptor();
908+
var constraintWeightSupplier = solutionDescriptor.getConstraintWeightSupplier();
899909
if (constraintWeightSupplier == null) {
900910
return false;
901911
}

core/src/main/java/ai/timefold/solver/core/impl/score/director/stream/BavetConstraintStreamScoreDirector.java

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,18 @@ private BavetConstraintStreamScoreDirector(
4343
// Complex methods
4444
// ************************************************************************
4545

46+
/**
47+
* The function is exclusively available for the Bavet score director, and its use must be approached with caution.
48+
* The primary purpose of this method is
49+
* to enable the {@code ConstraintVerifier}
50+
* to ignore events related to shadow variables when testing constraints that do not rely on them.
51+
*
52+
* @see AbstractScoreDirector#clearVariableListenerEvents()
53+
*/
54+
public void clearShadowVariablesListenerQueue() {
55+
clearVariableListenerEvents();
56+
}
57+
4658
@Override
4759
public void setWorkingSolution(Solution_ workingSolution) {
4860
session = scoreDirectorFactory.newSession(workingSolution, constraintMatchPolicy, derived);
@@ -53,7 +65,7 @@ public void setWorkingSolution(Solution_ workingSolution) {
5365
@Override
5466
public Score_ calculateScore() {
5567
variableListenerSupport.assertNotificationQueuesAreEmpty();
56-
Score_ score = session.calculateScore(getWorkingInitScore());
68+
var score = session.calculateScore(getWorkingInitScore());
5769
setCalculatedScore(score);
5870
return score;
5971
}
@@ -101,11 +113,11 @@ public void close() {
101113
@Override
102114
public void afterEntityAdded(EntityDescriptor<Solution_> entityDescriptor, Object entity) {
103115
if (entity == null) {
104-
throw new IllegalArgumentException("The entity (" + entity + ") cannot be added to the ScoreDirector.");
116+
throw new IllegalArgumentException("The entity (%s) cannot be added to the ScoreDirector.".formatted(entity));
105117
}
106118
if (!getSolutionDescriptor().hasEntityDescriptor(entity.getClass())) {
107-
throw new IllegalArgumentException("The entity (" + entity + ") of class (" + entity.getClass()
108-
+ ") is not a configured @" + PlanningEntity.class.getSimpleName() + ".");
119+
throw new IllegalArgumentException("The entity (%s) of class (%s) is not a configured @%s.".formatted(entity,
120+
entity.getClass(), PlanningEntity.class.getSimpleName()));
109121
}
110122
session.insert(entity);
111123
super.afterEntityAdded(entityDescriptor, entity);
@@ -143,7 +155,8 @@ public void afterEntityRemoved(EntityDescriptor<Solution_> entityDescriptor, Obj
143155
@Override
144156
public void afterProblemFactAdded(Object problemFact) {
145157
if (problemFact == null) {
146-
throw new IllegalArgumentException("The problemFact (" + problemFact + ") cannot be added to the ScoreDirector.");
158+
throw new IllegalArgumentException(
159+
"The problemFact (%s) cannot be added to the ScoreDirector.".formatted(problemFact));
147160
}
148161
session.insert(problemFact);
149162
super.afterProblemFactAdded(problemFact);

0 commit comments

Comments
 (0)