Skip to content

Commit 816b130

Browse files
authored
fix(constraint-streams): fact-based precompute() correctly propagates (#2338)
1 parent 868bd56 commit 816b130

2 files changed

Lines changed: 72 additions & 3 deletions

File tree

core/src/main/java/ai/timefold/solver/core/impl/bavet/common/RecordAndReplayPropagator.java

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ public final class RecordAndReplayPropagator<Tuple_ extends Tuple> implements Pr
4141
private final Supplier<BavetPrecomputeBuildHelper<Tuple_>> precomputeBuildHelperSupplier;
4242
private final UnaryOperator<Tuple_> internalTupleToOutputTupleMapper;
4343
private final Map<Object, List<Tuple_>> objectToOutputTuplesMap;
44+
/**
45+
* Output tuples which depend only on problem facts.
46+
* Unlike entity-derived output, these are not stored per-source
47+
* (facts never update, so they need no replay),
48+
* but they must still be delivered downstream exactly once.
49+
* Retained between recalculations so they can be retracted on cache invalidation.
50+
*/
51+
private final List<Tuple_> factOutputTupleList = new ArrayList<>();
4452
private final Set<Object> alreadyUpdatingSet = Collections.newSetFromMap(new IdentityHashMap<>());
4553
private final Map<Class<?>, Boolean> objectClassToIsEntitySourceClassMap;
4654

@@ -189,13 +197,19 @@ private void retractIfPresent(Tuple_ tuple) {
189197
}
190198

191199
private void invalidateCache() {
192-
objectToOutputTuplesMap.values().stream().flatMap(List::stream).forEach(this::retractIfPresent);
200+
objectToOutputTuplesMap.values()
201+
.stream()
202+
.flatMap(List::stream)
203+
.forEach(this::retractIfPresent);
193204
objectToOutputTuplesMap.clear();
205+
factOutputTupleList.forEach(this::retractIfPresent);
206+
factOutputTupleList.clear();
194207
}
195208

196209
private void recalculateTuples(NodeNetwork internalNodeNetwork, Map<Class<?>, List<BavetRootNode<?>>> classToRootNodeList,
197210
RecordingTupleLifecycle<Tuple_> recordingTupleLifecycle) {
198-
var internalTupleToOutputTupleMap = new IdentityHashMap<Tuple_, Tuple_>(seenEntitySet.size());
211+
var internalTupleToOutputTupleMap =
212+
new IdentityHashMap<Tuple_, Tuple_>(seenEntitySet.size() + seenFactSet.size());
199213
for (var invalidated : seenEntitySet) {
200214
var mappedTuples = new ArrayList<Tuple_>();
201215
try (var unusedActiveRecordingLifecycle = recordingTupleLifecycle.recordInto(
@@ -212,7 +226,28 @@ private void recalculateTuples(NodeNetwork internalNodeNetwork, Map<Class<?>, Li
212226
objectToOutputTuplesMap.put(invalidated, mappedTuples);
213227
}
214228
}
215-
objectToOutputTuplesMap.values().stream().flatMap(List::stream).forEach(this::insertIfAbsent);
229+
objectToOutputTuplesMap.values()
230+
.stream()
231+
.flatMap(List::stream)
232+
.forEach(this::insertIfAbsent);
233+
// Output tuples derived purely from facts are not re-emitted by any entity update above,
234+
// so they would never be delivered.
235+
// Facts never update during solving,
236+
// so a single recording pass over all facts is enough to capture every fact-dependent output tuple exactly once.
237+
// Tuples also derived from an entity were already delivered above;
238+
// insertIfAbsent skips them via tuple-state deduplication.
239+
if (!seenFactSet.isEmpty()) {
240+
try (var unusedActiveRecordingLifecycle =
241+
recordingTupleLifecycle.recordInto(new TupleRecorder<>(factOutputTupleList,
242+
internalTupleToOutputTupleMapper, internalTupleToOutputTupleMap))) {
243+
for (var fact : seenFactSet) {
244+
classToRootNodeList.get(fact.getClass())
245+
.forEach(node -> ((BavetRootNode<Object>) node).update(fact));
246+
}
247+
internalNodeNetwork.settle();
248+
}
249+
factOutputTupleList.forEach(this::insertIfAbsent);
250+
}
216251
}
217252

218253
}

core/src/test/java/ai/timefold/solver/core/impl/score/stream/common/uni/AbstractUniConstraintStreamPrecomputeTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,40 @@ protected AbstractUniConstraintStreamPrecomputeTest(ConstraintStreamImplSupport
3333
super(implSupport);
3434
}
3535

36+
/**
37+
* A precompute that simply enumerates a problem-fact class must emit one tuple per fact.
38+
* Removing a problem fact must re-derive the fact-only output
39+
* and retract the removed fact's tuple.
40+
* Exercises the cache-invalidation path for fact-derived output.
41+
*/
42+
@TestTemplate
43+
void forEachUnfiltered_fact() {
44+
var solution = TestdataLavishSolution.generateEmptySolution();
45+
var value1 = new TestdataLavishValue();
46+
var value2 = new TestdataLavishValue();
47+
var value3 = new TestdataLavishValue();
48+
solution.getValueList().addAll(List.of(value1, value2, value3));
49+
50+
var scoreDirector = buildScoreDirector(
51+
factory -> factory.precompute(pf -> pf.forEachUnfiltered(TestdataLavishValue.class))
52+
.penalize(SimpleScore.ONE)
53+
.asConstraint(TEST_CONSTRAINT_ID));
54+
55+
scoreDirector.setWorkingSolution(solution);
56+
assertScore(scoreDirector,
57+
assertMatch(value1),
58+
assertMatch(value2),
59+
assertMatch(value3));
60+
61+
scoreDirector.beforeProblemFactRemoved(value3);
62+
solution.getValueList().remove(value3);
63+
scoreDirector.afterProblemFactRemoved(value3);
64+
65+
assertScore(scoreDirector,
66+
assertMatch(value1),
67+
assertMatch(value2));
68+
}
69+
3670
@Override
3771
@TestTemplate
3872
public void filter_0_changed() {

0 commit comments

Comments
 (0)