Skip to content

Commit 72d48c2

Browse files
triceotriceo
authored
fix: centralize the ownership of ValueRangeManager to avoid subtle bugs (#1716)
`ValueRangeManager` is now owned and exclusively instantiated by `ScoreDirector`. Therefore all access to it is only possible within the context of a working solution. In order to achieve this, Move Streams get a special kind of filtering, with access to `SolutionView` - this will be useful anyway, since I've been meaning to do this at a later stage of Move Streams anyway.
1 parent c00494a commit 72d48c2

File tree

86 files changed

+1732
-1255
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

86 files changed

+1732
-1255
lines changed

‎core/src/main/java/ai/timefold/solver/core/impl/bavet/AbstractSession.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import ai.timefold.solver.core.api.domain.solution.PlanningSolution;
88
import ai.timefold.solver.core.impl.bavet.uni.AbstractForEachUniNode;
99
import ai.timefold.solver.core.impl.bavet.uni.AbstractForEachUniNode.LifecycleOperation;
10-
import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager;
10+
import ai.timefold.solver.core.impl.score.director.SessionContext;
1111

1212
public abstract class AbstractSession implements AutoCloseable {
1313

@@ -25,9 +25,10 @@ protected AbstractSession(NodeNetwork nodeNetwork) {
2525
this.retractEffectiveClassToNodeArrayMap = new IdentityHashMap<>(nodeNetwork.forEachNodeCount());
2626
}
2727

28-
public final void initialize(Object workingSolution, SupplyManager supplyManager) {
28+
@SuppressWarnings({ "unchecked", "rawtypes" })
29+
public final void initialize(SessionContext context) {
2930
for (var node : findInitializableNodes()) {
30-
node.initialize(workingSolution, supplyManager);
31+
node.initialize(context);
3132
}
3233
}
3334

‎core/src/main/java/ai/timefold/solver/core/impl/bavet/bi/joiner/DefaultBiJoiner.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public <Property_> DefaultBiJoiner(Function<A, Property_> leftMapping, JoinerTyp
2323
this.leftMappings = new Function[] { leftMapping };
2424
}
2525

26-
private <Property_> DefaultBiJoiner(Function<A, Property_>[] leftMappings, JoinerType[] joinerTypes,
26+
public <Property_> DefaultBiJoiner(Function<A, Property_>[] leftMappings, JoinerType[] joinerTypes,
2727
Function<B, Property_>[] rightMappings) {
2828
super(rightMappings, joinerTypes);
2929
this.leftMappings = leftMappings;

‎core/src/main/java/ai/timefold/solver/core/impl/bavet/uni/AbstractForEachUniNode.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import ai.timefold.solver.core.impl.bavet.common.tuple.TupleLifecycle;
1010
import ai.timefold.solver.core.impl.bavet.common.tuple.TupleState;
1111
import ai.timefold.solver.core.impl.bavet.common.tuple.UniTuple;
12-
import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager;
12+
import ai.timefold.solver.core.impl.score.director.SessionContext;
1313

1414
import org.jspecify.annotations.NullMarked;
1515
import org.jspecify.annotations.Nullable;
@@ -25,7 +25,7 @@
2525
@NullMarked
2626
public abstract sealed class AbstractForEachUniNode<A>
2727
extends AbstractNode
28-
permits ForEachExcludingUnassignedUniNode, ForEachExcludingPinnedUniNode, ForEachIncludingUnassignedUniNode {
28+
permits ForEachExcludingUnassignedUniNode, ForEachIncludingUnassignedUniNode {
2929

3030
private final Class<A> forEachClass;
3131
private final int outputStoreSize;
@@ -138,7 +138,7 @@ public enum LifecycleOperation {
138138

139139
public interface InitializableForEachNode<Solution_> extends AutoCloseable {
140140

141-
void initialize(Solution_ workingSolution, SupplyManager supplyManager);
141+
void initialize(SessionContext<Solution_> context);
142142

143143
@Override
144144
void close(); // Drop the checked exception.

‎core/src/main/java/ai/timefold/solver/core/impl/bavet/uni/ForEachExcludingPinnedUniNode.java

Lines changed: 0 additions & 101 deletions
This file was deleted.

‎core/src/main/java/ai/timefold/solver/core/impl/bavet/uni/ForEachFromSolutionUniNode.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@
55
import ai.timefold.solver.core.impl.bavet.common.tuple.TupleLifecycle;
66
import ai.timefold.solver.core.impl.bavet.common.tuple.UniTuple;
77
import ai.timefold.solver.core.impl.domain.valuerange.descriptor.ValueRangeDescriptor;
8-
import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager;
9-
import ai.timefold.solver.core.impl.score.director.ValueRangeManager;
8+
import ai.timefold.solver.core.impl.score.director.SessionContext;
109

1110
import org.jspecify.annotations.NullMarked;
1211
import org.jspecify.annotations.Nullable;
@@ -31,29 +30,26 @@ public final class ForEachFromSolutionUniNode<Solution_, A>
3130
extends ForEachIncludingUnassignedUniNode<A>
3231
implements AbstractForEachUniNode.InitializableForEachNode<Solution_> {
3332

34-
private final ValueRangeManager<Solution_> valueRangeManager;
3533
private final ValueRangeDescriptor<Solution_> valueRangeDescriptor;
3634

3735
private boolean isInitialized = false;
3836

3937
@SuppressWarnings("unchecked")
40-
public ForEachFromSolutionUniNode(ValueRangeManager<Solution_> valueRangeManager,
41-
ValueRangeDescriptor<Solution_> valueRangeDescriptor, TupleLifecycle<UniTuple<A>> nextNodesTupleLifecycle,
42-
int outputStoreSize) {
38+
public ForEachFromSolutionUniNode(ValueRangeDescriptor<Solution_> valueRangeDescriptor,
39+
TupleLifecycle<UniTuple<A>> nextNodesTupleLifecycle, int outputStoreSize) {
4340
super((Class<A>) valueRangeDescriptor.getVariableDescriptor().getVariablePropertyType(), nextNodesTupleLifecycle,
4441
outputStoreSize);
45-
this.valueRangeManager = Objects.requireNonNull(valueRangeManager);
4642
this.valueRangeDescriptor = Objects.requireNonNull(valueRangeDescriptor);
4743
}
4844

4945
@Override
50-
public void initialize(Solution_ workingSolution, SupplyManager supplyManager) {
46+
public void initialize(SessionContext<Solution_> context) {
5147
if (this.isInitialized) { // Failsafe.
5248
throw new IllegalStateException("Impossible state: initialize() has already been called on %s."
5349
.formatted(this));
5450
} else {
5551
this.isInitialized = true;
56-
var valueRange = valueRangeManager.<A> getFromSolution(valueRangeDescriptor, workingSolution);
52+
var valueRange = context.<A> getValueRange(valueRangeDescriptor);
5753
var valueIterator = valueRange.createOriginalIterator();
5854
while (valueIterator.hasNext()) {
5955
var value = valueIterator.next();

‎core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,21 @@ public void solve(SolverScope<Solution_> solverScope) {
5656
var phaseScope = buildPhaseScope(solverScope, phaseIndex);
5757
phaseStarted(phaseScope);
5858

59-
var solutionDescriptor = solverScope.getSolutionDescriptor();
6059
var hasListVariable = moveRepository.hasListVariable();
6160
var maxStepCount = -1;
6261
if (hasListVariable) {
6362
// In case of list variable with support for unassigned values, the placer will iterate indefinitely.
6463
// (When it exhausts all values, it will start over from the beginning.)
6564
// To prevent that, we need to limit the number of steps to the number of unassigned values.
66-
var workingSolution = phaseScope.getWorkingSolution();
67-
maxStepCount = solutionDescriptor.getListVariableDescriptor().countUnassigned(workingSolution,
68-
solverScope.getValueRangeManager());
65+
// The use of ValueRangeManager is safe
66+
// because it comes from the same score director as the working solution,
67+
// and therefore is guaranteed to match.
68+
// We compute fresh initialization statistics as opposed to possibly relying on a cached one,
69+
// as otherwise Ruin&Recreate doesn't work correctly with its nested CH phase.
70+
var scoreDirector = phaseScope.getScoreDirector();
71+
var valueRangeManager = scoreDirector.getValueRangeManager();
72+
maxStepCount = valueRangeManager.computeInitializationStatistics(scoreDirector.getWorkingSolution(), null)
73+
.notInAnyListValueCount();
6974
}
7075

7176
TerminationStatus earlyTerminationStatus = null;

‎core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseFactory.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor;
3333
import ai.timefold.solver.core.impl.heuristic.HeuristicConfigPolicy;
3434
import ai.timefold.solver.core.impl.phase.AbstractPhaseFactory;
35-
import ai.timefold.solver.core.impl.score.director.ValueRangeManager;
3635
import ai.timefold.solver.core.impl.solver.recaller.BestSolutionRecaller;
3736
import ai.timefold.solver.core.impl.solver.termination.PhaseTermination;
3837
import ai.timefold.solver.core.impl.solver.termination.SolverTermination;
@@ -80,7 +79,7 @@ protected DefaultConstructionHeuristicPhaseBuilder<Solution_> createBuilder(
8079
@Override
8180
public ConstructionHeuristicPhase<Solution_> buildPhase(int phaseIndex, boolean lastInitializingPhase,
8281
HeuristicConfigPolicy<Solution_> solverConfigPolicy, BestSolutionRecaller<Solution_> bestSolutionRecaller,
83-
SolverTermination<Solution_> solverTermination, ValueRangeManager<Solution_> valueRangeManager) {
82+
SolverTermination<Solution_> solverTermination) {
8483
return getBuilder(phaseIndex, lastInitializingPhase, solverConfigPolicy, solverTermination)
8584
.build();
8685
}

‎core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java

Lines changed: 13 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import ai.timefold.solver.core.impl.domain.common.ReflectionHelper;
4545
import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessor;
4646
import ai.timefold.solver.core.impl.domain.policy.DescriptorPolicy;
47-
import ai.timefold.solver.core.impl.domain.solution.descriptor.ProblemScaleTracker;
4847
import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor;
4948
import ai.timefold.solver.core.impl.domain.variable.anchor.AnchorShadowVariableDescriptor;
5049
import ai.timefold.solver.core.impl.domain.variable.cascade.CascadingUpdateShadowVariableDescriptor;
@@ -65,7 +64,8 @@
6564
import ai.timefold.solver.core.impl.heuristic.selector.common.decorator.ComparatorSelectionSorter;
6665
import ai.timefold.solver.core.impl.heuristic.selector.common.decorator.SelectionSorter;
6766
import ai.timefold.solver.core.impl.heuristic.selector.common.decorator.WeightFactorySelectionSorter;
68-
import ai.timefold.solver.core.impl.score.director.ValueRangeManager;
67+
import ai.timefold.solver.core.impl.move.director.MoveDirector;
68+
import ai.timefold.solver.core.impl.move.streams.maybeapi.UniDataFilter;
6969
import ai.timefold.solver.core.impl.util.CollectionUtils;
7070
import ai.timefold.solver.core.impl.util.MutableInt;
7171
import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningEntityMetaModel;
@@ -134,6 +134,12 @@ public class EntityDescriptor<Solution_> {
134134
private List<GenuineVariableDescriptor<Solution_>> effectiveGenuineVariableDescriptorList;
135135
private List<ListVariableDescriptor<Solution_>> effectiveGenuineListVariableDescriptorList;
136136

137+
private final UniDataFilter<Solution_, Object> entityMovablePredicate =
138+
(solutionView, entity) -> {
139+
var moveDirector = (MoveDirector<Solution_, ?>) solutionView;
140+
return !moveDirector.isPinned(this, entity);
141+
};
142+
137143
// Caches the metamodel
138144
private PlanningEntityMetaModel<Solution_, ?> entityMetaModel = null;
139145

@@ -647,10 +653,6 @@ public boolean hasEffectiveMovableEntityFilter() {
647653
return effectiveMovableEntityFilter != null;
648654
}
649655

650-
public boolean hasCascadingShadowVariables() {
651-
return !declaredShadowVariableDescriptorMap.isEmpty();
652-
}
653-
654656
public boolean supportsPinning() {
655657
return hasEffectiveMovableEntityFilter() || effectivePlanningPinToIndexReader != null;
656658
}
@@ -659,6 +661,11 @@ public BiPredicate<Solution_, Object> getEffectiveMovableEntityFilter() {
659661
return effectiveMovableEntityFilter;
660662
}
661663

664+
@SuppressWarnings("unchecked")
665+
public <A> UniDataFilter<Solution_, A> getEntityMovablePredicate() {
666+
return (UniDataFilter<Solution_, A>) entityMovablePredicate;
667+
}
668+
662669
public SelectionSorter<Solution_, Object> getDecreasingDifficultySorter() {
663670
return decreasingDifficultySorter;
664671
}
@@ -858,83 +865,6 @@ public PlanningPinToIndexReader getEffectivePlanningPinToIndexReader() {
858865
return effectivePlanningPinToIndexReader;
859866
}
860867

861-
public long getMaximumValueCount(Solution_ solution, Object entity, ValueRangeManager<Solution_> valueRangeManager) {
862-
var maximumValueCount = 0L;
863-
for (var variableDescriptor : effectiveGenuineVariableDescriptorList) {
864-
if (variableDescriptor.canExtractValueRangeFromSolution()) {
865-
maximumValueCount = Math.max(maximumValueCount,
866-
valueRangeManager.countOnSolution(variableDescriptor.getValueRangeDescriptor(),
867-
solution));
868-
} else {
869-
maximumValueCount = Math.max(maximumValueCount,
870-
valueRangeManager.countOnEntity(variableDescriptor.getValueRangeDescriptor(),
871-
entity));
872-
873-
}
874-
}
875-
return maximumValueCount;
876-
877-
}
878-
879-
public void processProblemScale(Solution_ solution, Object entity, ProblemScaleTracker tracker,
880-
ValueRangeManager<Solution_> valueRangeManager) {
881-
for (var variableDescriptor : effectiveGenuineVariableDescriptorList) {
882-
var valueCount = variableDescriptor.canExtractValueRangeFromSolution()
883-
? valueRangeManager.countOnSolution(variableDescriptor.getValueRangeDescriptor(),
884-
solution)
885-
: valueRangeManager.countOnEntity(variableDescriptor.getValueRangeDescriptor(), entity);
886-
// TODO: When minimum Java supported is 21, this can be replaced with a sealed interface switch
887-
if (variableDescriptor instanceof BasicVariableDescriptor<Solution_> basicVariableDescriptor) {
888-
if (basicVariableDescriptor.isChained()) {
889-
// An entity is a value
890-
tracker.addListValueCount(1);
891-
if (!isMovable(solution, entity)) {
892-
tracker.addPinnedListValueCount(1);
893-
}
894-
// Anchors are entities
895-
var valueRange = variableDescriptor.canExtractValueRangeFromSolution()
896-
? valueRangeManager.getFromSolution(variableDescriptor.getValueRangeDescriptor(),
897-
solution)
898-
: valueRangeManager.getFromEntity(variableDescriptor.getValueRangeDescriptor(),
899-
entity);
900-
var valueIterator = valueRange.createOriginalIterator();
901-
while (valueIterator.hasNext()) {
902-
var value = valueIterator.next();
903-
if (variableDescriptor.isValuePotentialAnchor(value)) {
904-
if (tracker.isAnchorVisited(value)) {
905-
continue;
906-
}
907-
// Assumes anchors are not pinned
908-
tracker.incrementListEntityCount(true);
909-
}
910-
}
911-
} else {
912-
if (isMovable(solution, entity)) {
913-
tracker.addBasicProblemScale(valueCount);
914-
}
915-
}
916-
} else if (variableDescriptor instanceof ListVariableDescriptor<Solution_> listVariableDescriptor) {
917-
var size = variableDescriptor.canExtractValueRangeFromSolution()
918-
? valueRangeManager.countOnSolution(listVariableDescriptor.getValueRangeDescriptor(),
919-
solution)
920-
: valueRangeManager.countOnEntity(listVariableDescriptor.getValueRangeDescriptor(),
921-
entity);
922-
tracker.setListTotalValueCount((int) size);
923-
if (isMovable(solution, entity)) {
924-
tracker.incrementListEntityCount(true);
925-
tracker.addPinnedListValueCount(listVariableDescriptor.getFirstUnpinnedIndex(entity));
926-
} else {
927-
tracker.incrementListEntityCount(false);
928-
tracker.addPinnedListValueCount(listVariableDescriptor.getListSize(entity));
929-
}
930-
} else {
931-
throw new IllegalStateException(
932-
"Unhandled subclass of %s encountered (%s).".formatted(VariableDescriptor.class.getSimpleName(),
933-
variableDescriptor.getClass().getSimpleName()));
934-
}
935-
}
936-
}
937-
938868
public int countUninitializedVariables(Object entity) {
939869
var count = 0;
940870
for (var variableDescriptor : effectiveGenuineVariableDescriptorList) {

0 commit comments

Comments
 (0)