From 09bd5572837ef8b5d92733c50b46e10262cd1ede Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Mon, 18 Aug 2025 19:27:09 -0400 Subject: [PATCH 1/6] chore: rename @ShadowVariableLooped to @ShadowVariableInconsistent - Solving will automatically update all shadow variables, so you can now pass in solutions with incorrect/stale shadow variables and they'll be correctly updated. - SolutionManager.update(SolutionUpdatePolicy.UPDATE_SCORE_ONLY, ...) will now fail fast if any @ShadowVariableInconsistent member is null or if any @InverseRelationVariable is null or incorrect. --- .../entity/descriptor/EntityDescriptor.java | 18 +- .../declarative/AffectedEntitiesUpdater.java | 10 +- .../DefaultShadowVariableSessionFactory.java | 5 +- .../variable/declarative/GraphStructure.java | 2 +- .../variable/declarative/LoopedTracker.java | 22 +- .../declarative/RootVariableSource.java | 10 +- ...iablesInconsistentVariableDescriptor.java} | 8 +- ...rectionalParentVariableReferenceGraph.java | 18 +- .../declarative/VariableUpdaterInfo.java | 10 +- ...verseRelationShadowVariableDescriptor.java | 4 + .../support/VariableListenerSupport.java | 120 +++++++ .../score/director/AbstractScoreDirector.java | 13 +- .../score/director/InnerScoreDirector.java | 10 + .../core/impl/solver/scope/SolverScope.java | 5 + ....java => ShadowVariablesInconsistent.java} | 14 +- .../core/api/solver/SolutionManagerTest.java | 321 ++++++++++++++++++ .../declarative/RootVariableSourceTest.java | 17 +- .../core/impl/solver/DefaultSolverTest.java | 79 +++++ .../ConcurrentValuesShadowVariableTest.java | 8 +- .../DependencyValuesShadowVariableTest.java | 4 +- .../basic/TestdataBasicVarValue.java | 4 +- .../chained/TestdataChainedVarValue.java | 4 +- ...ConcurrentAssertionConstraintProvider.java | 4 +- ...oncurrentAssertionEasyScoreCalculator.java | 2 +- .../TestdataConcurrentConstraintProvider.java | 4 +- ...TestdataConcurrentEasyScoreCalculator.java | 2 +- .../concurrent/TestdataConcurrentValue.java | 39 +-- .../dependency/TestdataDependencyValue.java | 4 +- .../TestdataInvalidDeclarativeValue.java | 14 +- ...stdataDeclarativeMissingSupplierValue.java | 4 +- ...TestdataMultiDirectionConcurrentValue.java | 4 +- .../TestdataMultiEntityDependencyValue.java | 4 +- ...dataInverseRelationConstraintProvider.java | 23 ++ ...ataInverseRelationEasyScoreCalculator.java | 18 + .../modeling-planning-problems.adoc | 32 +- .../solver/quarkus/deployment/DotNames.java | 8 +- .../gizmo/TestDataKitchenSinkEntity.java | 6 +- ...upplierVariableListConstraintProvider.java | 6 +- ...tdataQuarkusSupplierVariableListValue.java | 14 +- ...uarkusDeclarativeMissingSupplierValue.java | 4 +- 40 files changed, 747 insertions(+), 151 deletions(-) rename core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/{ShadowVariableLoopedVariableDescriptor.java => ShadowVariablesInconsistentVariableDescriptor.java} (88%) rename core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/{ShadowVariableLooped.java => ShadowVariablesInconsistent.java} (75%) create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/shadow/inverserelation/TestdataInverseRelationConstraintProvider.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/shadow/inverserelation/TestdataInverseRelationEasyScoreCalculator.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java index 3e10d3ff4d..f1920c48bf 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java @@ -52,7 +52,7 @@ import ai.timefold.solver.core.impl.domain.variable.custom.LegacyCustomShadowVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.custom.PiggybackShadowVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.declarative.DeclarativeShadowVariableDescriptor; -import ai.timefold.solver.core.impl.domain.variable.declarative.ShadowVariableLoopedVariableDescriptor; +import ai.timefold.solver.core.impl.domain.variable.declarative.ShadowVariablesInconsistentVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.BasicVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.GenuineVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor; @@ -70,7 +70,7 @@ import ai.timefold.solver.core.impl.util.CollectionUtils; import ai.timefold.solver.core.impl.util.MutableInt; import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningEntityMetaModel; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; @@ -95,7 +95,7 @@ public class EntityDescriptor { PiggybackShadowVariable.class, CustomShadowVariable.class, CascadingUpdateShadowVariable.class, - ShadowVariableLooped.class + ShadowVariablesInconsistent.class }; private static final Logger LOGGER = LoggerFactory.getLogger(EntityDescriptor.class); @@ -107,7 +107,7 @@ public class EntityDescriptor { private final Predicate isInitializedPredicate; private final List declaredPlanningPinIndexMemberAccessorList = new ArrayList<>(); @Nullable - private ShadowVariableLoopedVariableDescriptor shadowVariableLoopedDescriptor; + private ShadowVariablesInconsistentVariableDescriptor shadowVariablesInconsistentDescriptor; private Predicate hasNoNullVariablesBasicVar; private Predicate hasNoNullVariablesListVar; @@ -427,10 +427,10 @@ The entityClass (%s) has a @%s annotated member (%s) that has an unsupported typ declaredCascadingUpdateShadowVariableDecriptorMap.put(variableDescriptor.getTargetMethodName(), variableDescriptor); } - } else if (variableAnnotationClass.equals(ShadowVariableLooped.class)) { - var variableDescriptor = new ShadowVariableLoopedVariableDescriptor<>(nextVariableDescriptorOrdinal, this, + } else if (variableAnnotationClass.equals(ShadowVariablesInconsistent.class)) { + var variableDescriptor = new ShadowVariablesInconsistentVariableDescriptor<>(nextVariableDescriptorOrdinal, this, memberAccessor); - shadowVariableLoopedDescriptor = variableDescriptor; + shadowVariablesInconsistentDescriptor = variableDescriptor; declaredShadowVariableDescriptorMap.put(memberName, variableDescriptor); } else if (variableAnnotationClass.equals(PiggybackShadowVariable.class)) { var variableDescriptor = @@ -678,8 +678,8 @@ public GenuineVariableDescriptor getGenuineVariableDescriptor(String return effectiveGenuineVariableDescriptorMap.get(variableName); } - public @Nullable ShadowVariableLoopedVariableDescriptor getShadowVariableLoopedDescriptor() { - return shadowVariableLoopedDescriptor; + public @Nullable ShadowVariablesInconsistentVariableDescriptor getShadowVariablesInconsistentDescriptor() { + return shadowVariablesInconsistentDescriptor; } public boolean hasBothGenuineListAndBasicVariables() { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java index 4e6c6eb8d0..db9bd7f277 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java @@ -123,7 +123,7 @@ private void initializeChangeQueue(BitSet changed) { private boolean updateEntityShadowVariables(GraphNode entityVariable, boolean isVariableLooped) { var entity = entityVariable.entity(); var shadowVariableReferences = entityVariable.variableReferences(); - var loopDescriptor = shadowVariableReferences.get(0).shadowVariableLoopedDescriptor(); + var loopDescriptor = shadowVariableReferences.get(0).shadowVariablesInconsistentDescriptor(); var anyChanged = false; if (loopDescriptor != null) { @@ -151,9 +151,9 @@ private boolean updateEntityShadowVariables(GraphNode entityVariable, } private boolean updateLoopedStatusOfEntity(Object entity, int entityId, - ShadowVariableLoopedVariableDescriptor loopDescriptor) { - var oldLooped = (boolean) loopDescriptor.getValue(entity); - var isEntityLooped = loopedTracker.isEntityLooped(graph, entityId, oldLooped); + ShadowVariablesInconsistentVariableDescriptor loopDescriptor) { + var oldLooped = (Boolean) loopDescriptor.getValue(entity); + var isEntityLooped = loopedTracker.isEntityInconsistent(graph, entityId, oldLooped); if (!Objects.equals(oldLooped, isEntityLooped)) { changeShadowVariableAndNotify(loopDescriptor, entity, isEntityLooped); } @@ -161,7 +161,7 @@ private boolean updateLoopedStatusOfEntity(Object entity, int entityId, // Since an entity might correspond to multiple nodes, we want all nodes // for that entity to be marked as changed, not just the first node the // updater encounters - return loopedTracker.didEntityLoopedStatusChange(entityId); + return loopedTracker.didEntityInconsistentStatusChange(entityId); } private boolean updateShadowVariable(boolean isLooped, diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java index 7b9fba0c5d..56508193d4 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java @@ -263,7 +263,7 @@ public List> getUpdatersForEntityVariable(Object declarativeShadowVariableDescriptor.getVariableMetaModel(), updaterKey, declarativeShadowVariableDescriptor, - declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(), + declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariablesInconsistentDescriptor(), declarativeShadowVariableDescriptor.getMemberAccessor(), declarativeShadowVariableDescriptor.getCalculator()::executeGetter); if (declarativeShadowVariableDescriptor.getAlignmentKeyMap() != null) { @@ -354,7 +354,8 @@ private static VariableReferenceGraph buildArbitrarySingleEntityGrap variableId, variableIdToUpdaters.getNextId(), declarativeShadowVariableDescriptor, - declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(), + declarativeShadowVariableDescriptor.getEntityDescriptor() + .getShadowVariablesInconsistentDescriptor(), declarativeShadowVariableDescriptor.getMemberAccessor(), declarativeShadowVariableDescriptor.getCalculator()::executeGetter))); } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java index ca3d3e4df2..4abfd9b8d7 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java @@ -33,7 +33,7 @@ public enum GraphStructure { * a different entity is previous. This allows us * to use a successor function to find affected entities. * Since there is at most a single parent node, such a graph - * cannot be looped. + * cannot be inconsistent. */ SINGLE_DIRECTIONAL_PARENT, diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/LoopedTracker.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/LoopedTracker.java index ad8c118033..fe49540e0e 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/LoopedTracker.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/LoopedTracker.java @@ -7,6 +7,7 @@ import ai.timefold.solver.core.impl.util.DynamicIntArray; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; @NullMarked public final class LoopedTracker { @@ -21,11 +22,11 @@ public final class LoopedTracker { // Needed so we can update all nodes in a cycle or depended on a cycle // (only nodes that formed a cycle get marked as changed by the graph; // their dependents don't) - private final boolean[] entityLoopedStatusChanged; + private final boolean[] entityInconsistentStatusChanged; public LoopedTracker(int nodeCount, int[][] entityIdToNodes) { this.entityIdToNodes = entityIdToNodes; - this.entityLoopedStatusChanged = new boolean[entityIdToNodes.length]; + this.entityInconsistentStatusChanged = new boolean[entityIdToNodes.length]; // We never fully clear the array, as that was shown to cause too much GC pressure. this.looped = new DynamicIntArray(nodeCount, PARTIAL); } @@ -34,23 +35,24 @@ public void mark(int node, LoopedStatus status) { looped.set(node, status.ordinal()); } - public boolean isEntityLooped(BaseTopologicalOrderGraph graph, int entityId, boolean wasEntityLooped) { + public boolean isEntityInconsistent(BaseTopologicalOrderGraph graph, int entityId, + @Nullable Boolean wasEntityInconsistent) { for (var entityNode : entityIdToNodes[entityId]) { if (graph.isLooped(this, entityNode)) { - if (!wasEntityLooped) { - entityLoopedStatusChanged[entityId] = true; + if (wasEntityInconsistent == null || !wasEntityInconsistent) { + entityInconsistentStatusChanged[entityId] = true; } return true; } } - if (wasEntityLooped) { - entityLoopedStatusChanged[entityId] = true; + if (wasEntityInconsistent == null || wasEntityInconsistent) { + entityInconsistentStatusChanged[entityId] = true; } return false; } - public boolean didEntityLoopedStatusChange(int entityId) { - return entityLoopedStatusChanged[entityId]; + public boolean didEntityInconsistentStatusChange(int entityId) { + return entityInconsistentStatusChanged[entityId]; } public LoopedStatus status(int node) { @@ -60,7 +62,7 @@ public LoopedStatus status(int node) { } public void clear() { - Arrays.fill(entityLoopedStatusChanged, false); + Arrays.fill(entityInconsistentStatusChanged, false); looped.clear(); } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java index 53c223361c..979903174c 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java @@ -23,7 +23,7 @@ import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningSolutionMetaModel; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; @@ -396,16 +396,16 @@ private static ParentVariableType determineParentVariableType(Class rootClass return ParentVariableType.VARIABLE; } - if (getAnnotation(declaringClass, memberName, ShadowVariableLooped.class) != null) { + if (getAnnotation(declaringClass, memberName, ShadowVariablesInconsistent.class) != null) { throw new IllegalArgumentException(""" The source path (%s) starting from root class (%s) accesses a @%s property (%s). - Supplier methods are only called when none of their dependencies are looped, + Supplier methods are only called when none of their dependencies are inconsistent, so reading @%s properties are not needed since they are guaranteed to be false when the supplier is called. Maybe remove the source path (%s) from the @%s? """.formatted( - variablePath, rootClass.getCanonicalName(), ShadowVariableLooped.class.getSimpleName(), - memberName, ShadowVariableLooped.class.getSimpleName(), + variablePath, rootClass.getCanonicalName(), ShadowVariablesInconsistent.class.getSimpleName(), + memberName, ShadowVariablesInconsistent.class.getSimpleName(), variablePath, ShadowSources.class.getSimpleName())); } return ParentVariableType.NO_PARENT; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ShadowVariableLoopedVariableDescriptor.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ShadowVariablesInconsistentVariableDescriptor.java similarity index 88% rename from core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ShadowVariableLoopedVariableDescriptor.java rename to core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ShadowVariablesInconsistentVariableDescriptor.java index 18921a0385..0273287b4a 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ShadowVariableLoopedVariableDescriptor.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ShadowVariablesInconsistentVariableDescriptor.java @@ -15,10 +15,10 @@ import ai.timefold.solver.core.impl.domain.variable.listener.VariableListenerWithSources; import ai.timefold.solver.core.impl.domain.variable.supply.Demand; import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; -public class ShadowVariableLoopedVariableDescriptor extends ShadowVariableDescriptor { - public ShadowVariableLoopedVariableDescriptor(int ordinal, +public class ShadowVariablesInconsistentVariableDescriptor extends ShadowVariableDescriptor { + public ShadowVariablesInconsistentVariableDescriptor(int ordinal, EntityDescriptor entityDescriptor, MemberAccessor variableMemberAccessor) { super(ordinal, entityDescriptor, variableMemberAccessor); @@ -33,7 +33,7 @@ The member (%s) on the entity class (%s) has an (%s) annotation, but the declara Maybe enable declarative shadow variables in your %s? """ .formatted(variableMemberAccessor.getName(), entityDescriptor.getEntityClass().getName(), - ShadowVariableLooped.class.getSimpleName(), SolverConfig.class.getSimpleName())); + ShadowVariablesInconsistent.class.getSimpleName(), SolverConfig.class.getSimpleName())); } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java index 6c124bced6..ae2247b045 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java @@ -41,8 +41,9 @@ public SingleDirectionalParentVariableReferenceGraph( this.changedVariableNotifier = changedVariableNotifier; var shadowEntities = Arrays.stream(entities).filter(monitoredEntityClass::isInstance) .sorted(topologicalOrderComparator).toArray(); - var loopedDescriptor = - sortedDeclarativeShadowVariableDescriptors.get(0).getEntityDescriptor().getShadowVariableLoopedDescriptor(); + var inconsistentDescriptor = + sortedDeclarativeShadowVariableDescriptors.get(0).getEntityDescriptor() + .getShadowVariablesInconsistentDescriptor(); var updaterIndex = 0; for (var variableDescriptor : sortedDeclarativeShadowVariableDescriptors) { @@ -51,7 +52,7 @@ public SingleDirectionalParentVariableReferenceGraph( variableMetaModel, updaterIndex, variableDescriptor, - loopedDescriptor, + inconsistentDescriptor, variableDescriptor.getMemberAccessor(), variableDescriptor.getCalculator()::executeGetter); sortedVariableUpdaterInfos[updaterIndex++] = variableUpdaterInfo; @@ -64,15 +65,16 @@ public SingleDirectionalParentVariableReferenceGraph( } changedEntities.addAll(List.of(shadowEntities)); - updateChanged(); - if (loopedDescriptor != null) { + if (inconsistentDescriptor != null) { for (var shadowEntity : shadowEntities) { - changedVariableNotifier.beforeVariableChanged().accept(loopedDescriptor, shadowEntity); - loopedDescriptor.setValue(shadowEntity, false); - changedVariableNotifier.afterVariableChanged().accept(loopedDescriptor, shadowEntity); + changedVariableNotifier.beforeVariableChanged().accept(inconsistentDescriptor, shadowEntity); + inconsistentDescriptor.setValue(shadowEntity, false); + changedVariableNotifier.afterVariableChanged().accept(inconsistentDescriptor, shadowEntity); } } + + updateChanged(); } @Override diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java index e823c174ea..0364f7161c 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java @@ -15,7 +15,7 @@ public record VariableUpdaterInfo( VariableMetaModel id, int groupId, DeclarativeShadowVariableDescriptor variableDescriptor, - @Nullable ShadowVariableLoopedVariableDescriptor shadowVariableLoopedDescriptor, + @Nullable ShadowVariablesInconsistentVariableDescriptor shadowVariablesInconsistentDescriptor, MemberAccessor memberAccessor, Function calculator, @Nullable Object[] groupEntities) { @@ -23,19 +23,19 @@ public record VariableUpdaterInfo( public VariableUpdaterInfo(VariableMetaModel id, int groupId, DeclarativeShadowVariableDescriptor variableDescriptor, - @Nullable ShadowVariableLoopedVariableDescriptor shadowVariableLoopedDescriptor, + @Nullable ShadowVariablesInconsistentVariableDescriptor shadowVariablesInconsistentDescriptor, MemberAccessor memberAccessor, Function calculator) { - this(id, groupId, variableDescriptor, shadowVariableLoopedDescriptor, memberAccessor, calculator, null); + this(id, groupId, variableDescriptor, shadowVariablesInconsistentDescriptor, memberAccessor, calculator, null); } public VariableUpdaterInfo withGroupId(int groupId) { - return new VariableUpdaterInfo<>(id, groupId, variableDescriptor, shadowVariableLoopedDescriptor, memberAccessor, + return new VariableUpdaterInfo<>(id, groupId, variableDescriptor, shadowVariablesInconsistentDescriptor, memberAccessor, calculator, groupEntities); } public VariableUpdaterInfo withGroupEntities(Object[] groupEntities) { - return new VariableUpdaterInfo<>(id, groupId, variableDescriptor, shadowVariableLoopedDescriptor, memberAccessor, + return new VariableUpdaterInfo<>(id, groupId, variableDescriptor, shadowVariablesInconsistentDescriptor, memberAccessor, calculator, groupEntities); } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/inverserelation/InverseRelationShadowVariableDescriptor.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/inverserelation/InverseRelationShadowVariableDescriptor.java index d29ca6f62e..53af81b47f 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/inverserelation/InverseRelationShadowVariableDescriptor.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/inverserelation/InverseRelationShadowVariableDescriptor.java @@ -183,6 +183,10 @@ private AbstractVariableListener buildVariableListener() { } } + public boolean isSingleton() { + return singleton; + } + @Override public boolean isListVariableSource() { return sourceVariableDescriptor instanceof ListVariableDescriptor; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java index a3f5870594..63d98db738 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java @@ -17,6 +17,7 @@ import java.util.function.IntFunction; import ai.timefold.solver.core.api.domain.solution.PlanningSolution; +import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; import ai.timefold.solver.core.api.score.director.ScoreDirector; import ai.timefold.solver.core.enterprise.TimefoldSolverEnterpriseService; import ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptor; @@ -40,6 +41,7 @@ import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager; import ai.timefold.solver.core.impl.score.director.InnerScoreDirector; import ai.timefold.solver.core.impl.util.LinkedIdentityHashSet; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; @@ -229,6 +231,45 @@ public void resetWorkingSolution() { notifiable.resetWorkingSolution(); } + if (scoreDirector.expectShadowVariablesInCorrectState()) { + var solutionDescriptor = scoreDirector.getSolutionDescriptor(); + solutionDescriptor.visitAllEntities(scoreDirector.getWorkingSolution(), + entity -> { + var entityDescriptors = solutionDescriptor.getEntityDescriptors(); + + for (var entityDescriptor : entityDescriptors) { + if (!entityDescriptor.getEntityClass().isInstance(entity)) { + continue; + } + var inconsistentShadowVariablesDescriptor = + entityDescriptor.getShadowVariablesInconsistentDescriptor(); + if (inconsistentShadowVariablesDescriptor != null + && inconsistentShadowVariablesDescriptor.getValue(entity) == null) { + throw new IllegalStateException( + """ + Shadow variables update is disabled, but the entity (%s) has a null @%s annotated field (%s). + Maybe enable shadow variable updates? + """ + .formatted(entity, ShadowVariablesInconsistent.class.getSimpleName(), + inconsistentShadowVariablesDescriptor.getVariableName())); + } + + var shadowVariableDescriptors = entityDescriptor.getShadowVariableDescriptors(); + for (var shadowVariableDescriptor : shadowVariableDescriptors) { + if (shadowVariableDescriptor instanceof InverseRelationShadowVariableDescriptor inverseDescriptor) { + if (inverseDescriptor.isSingleton()) { + assertSingletonInverseDescriptor(inverseDescriptor, scoreDirector.getWorkingSolution(), + entity); + } else { + assertCollectionInverseDescriptor(inverseDescriptor, scoreDirector.getWorkingSolution(), + entity); + } + } + } + } + }); + } + if (!scoreDirector.getSolutionDescriptor().getDeclarativeShadowVariableDescriptors().isEmpty()) { var shadowVariableSessionFactory = new DefaultShadowVariableSessionFactory<>( scoreDirector.getSolutionDescriptor(), @@ -239,6 +280,85 @@ public void resetWorkingSolution() { } } + private void assertSingletonInverseDescriptor(InverseRelationShadowVariableDescriptor inverseDescriptor, + Solution_ workingSolution, Object shadowEntity) { + var inverse = inverseDescriptor.getValue(shadowEntity); + var sourceDescriptor = inverseDescriptor.getSourceVariableDescriptorList().get(0); + if (sourceDescriptor.isListVariable()) { + return; // ExternalizedListInverseVariableProcessor asserts this in setInverseAsserted + } + if (inverse == null) { + scoreDirector.getSolutionDescriptor().visitAllEntities(workingSolution, maybeSourceEntity -> { + if (sourceDescriptor.getEntityDescriptor().getEntityClass().isInstance(maybeSourceEntity)) { + var sourceValue = sourceDescriptor.getValue(maybeSourceEntity); + if (sourceValue == shadowEntity) { + throw new IllegalStateException(""" + The entity (%s) has a @%s that is null, but there is a source entity (%s) that point to it. + Verify the consistency of your input solution. + """.formatted(shadowEntity, InverseRelationShadowVariable.class.getSimpleName(), + maybeSourceEntity)); + } + } + }); + } else { + var sourceValue = sourceDescriptor.getValue(inverse); + if (sourceValue != shadowEntity) { + throw new IllegalStateException( + """ + The entity (%s) has a @%s that points to a source entity (%s) but that source entity points to (%s) instead. + Verify the consistency of your input solution. + """ + .formatted(shadowEntity, InverseRelationShadowVariable.class.getSimpleName(), inverse, + sourceValue)); + } + } + } + + private void assertCollectionInverseDescriptor(InverseRelationShadowVariableDescriptor inverseDescriptor, + Solution_ workingSolution, Object shadowEntity) { + var inverseCollection = (Collection) inverseDescriptor.getValue(shadowEntity); + var sourceDescriptor = inverseDescriptor.getSourceVariableDescriptorList().get(0); + if (inverseCollection == null) { + throw new IllegalStateException(""" + The entity (%s) has a collection @%s that is null. + Verify the consistency of your input solution. + """.formatted(shadowEntity, InverseRelationShadowVariable.class.getSimpleName())); + } else { + for (var source : inverseCollection) { + var sourceValue = sourceDescriptor.getValue(source); + if (sourceValue != shadowEntity) { + throw new IllegalStateException( + """ + The entity (%s) has a collection @%s (%s) that points to a source entity (%s) but that source entity points to (%s) instead. + Verify the consistency of your input solution. + """ + .formatted(shadowEntity, InverseRelationShadowVariable.class.getSimpleName(), + inverseCollection, + source, sourceValue)); + } + } + + scoreDirector.getSolutionDescriptor().visitAllEntities(workingSolution, maybeSourceEntity -> { + if (sourceDescriptor.getEntityDescriptor().getEntityClass().isInstance(maybeSourceEntity)) { + if (inverseCollection.contains(maybeSourceEntity)) { + return; + } + var sourceValue = sourceDescriptor.getValue(maybeSourceEntity); + if (sourceValue == shadowEntity) { + throw new IllegalStateException( + """ + The entity (%s) has a collection @%s (%s), but it is missing a source entity that point to it (%s). + Verify the consistency of your input solution. + """ + .formatted(shadowEntity, InverseRelationShadowVariable.class.getSimpleName(), + inverseCollection, + maybeSourceEntity)); + } + } + }); + } + } + public void close() { for (var notifiable : notifiableRegistry.getAll()) { notifiable.closeVariableListener(); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java index eb2d1394c9..7bc54592be 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java @@ -68,11 +68,12 @@ public abstract class AbstractScoreDirector variableDescriptorCache; protected final VariableListenerSupport variableListenerSupport; + private boolean expectShadowVariablesInCorrectState; + private long workingEntityListRevision = 0L; private int workingGenuineEntityCount = 0; private boolean allChangesWillBeUndoneBeforeStepEnds = false; @@ -159,6 +160,16 @@ public boolean expectShadowVariablesInCorrectState() { return expectShadowVariablesInCorrectState; } + @Override + public void enableShadowVariablesInCorrectStateChecks() { + expectShadowVariablesInCorrectState = true; + } + + @Override + public void disableShadowVariablesInCorrectStateChecks() { + expectShadowVariablesInCorrectState = false; + } + @Override public @NonNull Solution_ getWorkingSolution() { return workingSolution; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java index 503edbba3e..aed250cf49 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java @@ -209,6 +209,16 @@ static > ConstraintAnalysis getConstraintAn */ boolean expectShadowVariablesInCorrectState(); + /** + * Disables the checks controlled by {@link #expectShadowVariablesInCorrectState()}. + */ + void disableShadowVariablesInCorrectStateChecks(); + + /** + * Enables the checks controlled by {@link #expectShadowVariablesInCorrectState()}. + */ + void enableShadowVariablesInCorrectStateChecks(); + /** * @return never null */ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java index 5e5e05b5ba..541afdf86c 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java @@ -332,7 +332,12 @@ public long getMoveEvaluationSpeed() { public void setWorkingSolutionFromBestSolution() { // The workingSolution must never be the same instance as the bestSolution. + scoreDirector.disableShadowVariablesInCorrectStateChecks(); scoreDirector.setWorkingSolution(scoreDirector.cloneSolution(getBestSolution())); + // The best solution might have some stale shadow variables, so + // update all shadow variables + scoreDirector.forceTriggerVariableListeners(); + scoreDirector.enableShadowVariablesInCorrectStateChecks(); } public SolverScope createChildThreadSolverScope(ChildThreadType childThreadType) { diff --git a/core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/ShadowVariableLooped.java b/core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/ShadowVariablesInconsistent.java similarity index 75% rename from core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/ShadowVariableLooped.java rename to core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/ShadowVariablesInconsistent.java index 76054b9e66..a6a2d4f915 100644 --- a/core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/ShadowVariableLooped.java +++ b/core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/ShadowVariablesInconsistent.java @@ -15,9 +15,9 @@ /** * Specifies that a boolean property (or field) of a {@link PlanningEntity} * tracks if any of its {@link ShadowVariable#supplierName() supplier variables} - * are looped. + * are inconsistent. *

- * A supplier variable is looped if: + * A supplier variable is inconsistent if: *

    *
  • * One of its source variables include it as a source (for example, @@ -30,18 +30,18 @@ *
*

* Should be used in a filter for a hard {@link Constraint} to penalize - * looped entities, since {@link PlanningSolution} with looped entities are + * inconsistent entities, since {@link PlanningSolution} with inconsistent entities are * typically not valid. *

* Important: - * Do not use a {@link ShadowVariableLooped} property in a method annotated - * with {@link ShadowSources}. {@link ShadowVariableLooped} properties can + * Do not use a {@link ShadowVariablesInconsistent} property in a method annotated + * with {@link ShadowSources}. {@link ShadowVariablesInconsistent} properties can * be updated after the {@link ShadowSources} marked method is called, causing * score corruption. {@link ShadowSources} marked methods do not need to check - * {@link ShadowVariableLooped} properties, since they are only called if all + * {@link ShadowVariablesInconsistent} properties, since they are only called if all * their dependencies are not looped. */ @Target({ METHOD, FIELD }) @Retention(RUNTIME) -public @interface ShadowVariableLooped { +public @interface ShadowVariablesInconsistent { } diff --git a/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java b/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java index dfa7e87909..f3260757f6 100644 --- a/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java +++ b/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java @@ -1,6 +1,7 @@ package ai.timefold.solver.core.api.solver; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.SoftAssertions.assertSoftly; @@ -8,9 +9,12 @@ import java.util.List; import java.util.function.Function; +import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; import ai.timefold.solver.core.api.score.Score; +import ai.timefold.solver.core.api.score.buildin.hardsoft.HardSoftScore; import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore; import ai.timefold.solver.core.config.score.director.ScoreDirectorFactoryConfig; +import ai.timefold.solver.core.config.solver.PreviewFeature; import ai.timefold.solver.core.config.solver.SolverConfig; import ai.timefold.solver.core.impl.solver.DefaultSolutionManager; import ai.timefold.solver.core.impl.util.Pair; @@ -23,6 +27,10 @@ import ai.timefold.solver.core.testdomain.chained.shadow.TestdataShadowingChainedIncrementalScoreCalculator; import ai.timefold.solver.core.testdomain.chained.shadow.TestdataShadowingChainedObject; import ai.timefold.solver.core.testdomain.chained.shadow.TestdataShadowingChainedSolution; +import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentConstraintProvider; +import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentEntity; +import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentSolution; +import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentValue; import ai.timefold.solver.core.testdomain.list.pinned.index.TestdataPinnedWithIndexListCMAIncrementalScoreCalculator; import ai.timefold.solver.core.testdomain.list.pinned.index.TestdataPinnedWithIndexListEntity; import ai.timefold.solver.core.testdomain.list.pinned.index.TestdataPinnedWithIndexListSolution; @@ -38,6 +46,10 @@ import ai.timefold.solver.core.testdomain.shadow.TestdataShadowedEntity; import ai.timefold.solver.core.testdomain.shadow.TestdataShadowedIncrementalScoreCalculator; import ai.timefold.solver.core.testdomain.shadow.TestdataShadowedSolution; +import ai.timefold.solver.core.testdomain.shadow.inverserelation.TestdataInverseRelationConstraintProvider; +import ai.timefold.solver.core.testdomain.shadow.inverserelation.TestdataInverseRelationEntity; +import ai.timefold.solver.core.testdomain.shadow.inverserelation.TestdataInverseRelationSolution; +import ai.timefold.solver.core.testdomain.shadow.inverserelation.TestdataInverseRelationValue; import ai.timefold.solver.core.testdomain.unassignedvar.TestdataAllowsUnassignedEntity; import ai.timefold.solver.core.testdomain.unassignedvar.TestdataAllowsUnassignedIncrementalScoreCalculator; import ai.timefold.solver.core.testdomain.unassignedvar.TestdataAllowsUnassignedSolution; @@ -59,6 +71,12 @@ public class SolutionManagerTest { .withScoreDirectorFactory( new ScoreDirectorFactoryConfig().withIncrementalScoreCalculatorClass( TestdataShadowedIncrementalScoreCalculator.class))); + public static final SolverFactory SOLVER_FACTORY_DECLARATIVE_SHADOW = SolverFactory.create( + new SolverConfig() + .withSolutionClass(TestdataConcurrentSolution.class) + .withEntityClasses(TestdataConcurrentEntity.class, TestdataConcurrentValue.class) + .withConstraintProviderClass(TestdataConcurrentConstraintProvider.class) + .withPreviewFeature(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES)); public static final SolverFactory SOLVER_FACTORY_UNASSIGNED = SolverFactory.create( new SolverConfig() .withSolutionClass(TestdataAllowsUnassignedSolution.class) @@ -99,6 +117,11 @@ public class SolutionManagerTest { .withSolutionClass(TestdataSolution.class) .withEntityClasses(TestdataEntity.class) .withConstraintProviderClass(TestdataConstraintProvider.class)); + public static final SolverFactory SOLVER_FACTORY_INVERSE_RELATION = SolverFactory.create( + new SolverConfig() + .withSolutionClass(TestdataInverseRelationSolution.class) + .withEntityClasses(TestdataInverseRelationEntity.class, TestdataInverseRelationValue.class) + .withConstraintProviderClass(TestdataInverseRelationConstraintProvider.class)); @ParameterizedTest @EnumSource(SolutionManagerSource.class) @@ -262,6 +285,304 @@ void updateOnlyScore(SolutionManagerSource SolutionManagerSource) { }); } + @ParameterizedTest + @EnumSource(SolutionManagerSource.class) + void updateOnlyScoreDeclarativeShadows(SolutionManagerSource SolutionManagerSource) { + var solution = new TestdataConcurrentSolution(); + var e1 = new TestdataConcurrentEntity("e1"); + var e2 = new TestdataConcurrentEntity("e2"); + + var a1 = new TestdataConcurrentValue("a1"); + var a2 = new TestdataConcurrentValue("a2"); + var b1 = new TestdataConcurrentValue("b1"); + var b2 = new TestdataConcurrentValue("b2"); + + var groupA = List.of(a1, a2); + var groupB = List.of(b1, b2); + + var entities = List.of(e1, e2); + var values = List.of(a1, a2, b1, b2); + + a1.setConcurrentValueGroup(groupA); + a2.setConcurrentValueGroup(groupA); + + b1.setConcurrentValueGroup(groupB); + b2.setConcurrentValueGroup(groupB); + + e1.setValues(List.of(a1, b1)); + e2.setValues(List.of(b2, a2)); + + b1.setPreviousValue(a1); + a2.setPreviousValue(b2); + + a1.setNextValue(b1); + b2.setNextValue(a2); + + a1.setIndex(0); + a2.setIndex(1); + + b1.setIndex(1); + b2.setIndex(0); + + a1.setEntity(e1); + b1.setEntity(e1); + a2.setEntity(e2); + b2.setEntity(e2); + + a1.setInconsistent(true); + a2.setInconsistent(true); + b1.setInconsistent(true); + b2.setInconsistent(true); + + solution.setEntities(entities); + solution.setValues(values); + + var solutionManager = SolutionManagerSource.createSolutionManager(SOLVER_FACTORY_DECLARATIVE_SHADOW); + var score = solutionManager.update(solution, SolutionUpdatePolicy.UPDATE_SCORE_ONLY); + assertThat(score).isEqualTo(HardSoftScore.ofHard(-4)); + assertThat(solution.getScore()).isEqualTo(HardSoftScore.ofHard(-4)); + } + + @ParameterizedTest + @EnumSource(SolutionManagerSource.class) + void updateOnlyScoreFailsIfShadowVariablesInconsistentIsNull(SolutionManagerSource SolutionManagerSource) { + var solution = new TestdataConcurrentSolution(); + var e1 = new TestdataConcurrentEntity("e1"); + var e2 = new TestdataConcurrentEntity("e2"); + + var a1 = new TestdataConcurrentValue("a1"); + var a2 = new TestdataConcurrentValue("a2"); + var b1 = new TestdataConcurrentValue("b1"); + var b2 = new TestdataConcurrentValue("b2"); + + var groupA = List.of(a1, a2); + var groupB = List.of(b1, b2); + + var entities = List.of(e1, e2); + var values = List.of(a1, a2, b1, b2); + + a1.setConcurrentValueGroup(groupA); + a2.setConcurrentValueGroup(groupA); + + b1.setConcurrentValueGroup(groupB); + b2.setConcurrentValueGroup(groupB); + + e1.setValues(List.of(a1, b1)); + e2.setValues(List.of(b2, a2)); + + b1.setPreviousValue(a1); + a2.setPreviousValue(b2); + + a1.setNextValue(b1); + b2.setNextValue(a2); + + a1.setIndex(0); + a2.setIndex(1); + + b1.setIndex(1); + b2.setIndex(0); + + a1.setEntity(e1); + b1.setEntity(e1); + a2.setEntity(e2); + b2.setEntity(e2); + + a1.setInconsistent(true); + a2.setInconsistent(null); + b1.setInconsistent(true); + b2.setInconsistent(true); + + solution.setEntities(entities); + solution.setValues(values); + + var solutionManager = SolutionManagerSource.createSolutionManager(SOLVER_FACTORY_DECLARATIVE_SHADOW); + assertThat(solutionManager).isNotNull(); + assertThatCode(() -> { + solutionManager.update(solution, SolutionUpdatePolicy.UPDATE_SCORE_ONLY); + }).hasMessageContainingAll( + "Shadow variables update is disabled", + "but the entity (e2 -> b2 -> a2)", + "has a null @ShadowVariablesInconsistent annotated field (isInconsistent)"); + } + + @ParameterizedTest + @EnumSource(SolutionManagerSource.class) + void updateOnlyScoreFailsIfInverseCollectionNull(SolutionManagerSource SolutionManagerSource) { + var solution = new TestdataInverseRelationSolution("solution"); + var e1 = new TestdataInverseRelationEntity("e1"); + var e2 = new TestdataInverseRelationEntity("e2"); + var e3 = new TestdataInverseRelationEntity("e3"); + + var v1 = new TestdataInverseRelationValue("v1"); + var v2 = new TestdataInverseRelationValue("v2"); + + v1.setEntities(null); + v2.setEntities(null); + + solution.setEntityList(List.of(e1, e2, e3)); + solution.setValueList(List.of(v1, v2)); + + e1.setValue(v1); + e2.setValue(v1); + e3.setValue(v2); + + assertSoftly(softly -> { + softly.assertThat(solution.getScore()).isNull(); + softly.assertThat(solution.getValueList().get(0).getEntities()).isNull(); + softly.assertThat(solution.getValueList().get(1).getEntities()).isNull(); + }); + + var solutionManager = SolutionManagerSource.createSolutionManager(SOLVER_FACTORY_INVERSE_RELATION); + assertThat(solutionManager).isNotNull(); + assertThatCode(() -> { + solutionManager.update(solution, SolutionUpdatePolicy.UPDATE_SCORE_ONLY); + }).hasMessageContainingAll( + "The entity (v1)", + "has a collection @" + InverseRelationShadowVariable.class.getSimpleName(), + "that is null"); + } + + @ParameterizedTest + @EnumSource(SolutionManagerSource.class) + void updateOnlyScoreFailsIfInverseCollectionMissingElements(SolutionManagerSource SolutionManagerSource) { + var solution = new TestdataInverseRelationSolution("solution"); + var e1 = new TestdataInverseRelationEntity("e1"); + var e2 = new TestdataInverseRelationEntity("e2"); + var e3 = new TestdataInverseRelationEntity("e3"); + + var v1 = new TestdataInverseRelationValue("v1"); + var v2 = new TestdataInverseRelationValue("v2"); + + v1.setEntities(List.of(e1)); + v2.setEntities(List.of(e3)); + + solution.setEntityList(List.of(e1, e2, e3)); + solution.setValueList(List.of(v1, v2)); + + e1.setValue(v1); + e2.setValue(v1); + e3.setValue(v2); + + assertSoftly(softly -> { + softly.assertThat(solution.getScore()).isNull(); + softly.assertThat(solution.getValueList().get(0).getEntities()).containsExactly(e1); + softly.assertThat(solution.getValueList().get(1).getEntities()).containsExactly(e3); + }); + + var solutionManager = SolutionManagerSource.createSolutionManager(SOLVER_FACTORY_INVERSE_RELATION); + assertThat(solutionManager).isNotNull(); + assertThatCode(() -> { + solutionManager.update(solution, SolutionUpdatePolicy.UPDATE_SCORE_ONLY); + }).hasMessageContainingAll( + "The entity (v1)", + "has a collection @" + InverseRelationShadowVariable.class.getSimpleName(), + "([e1])", + "but it is missing a source entity that point to it (e2)"); + } + + @ParameterizedTest + @EnumSource(SolutionManagerSource.class) + void updateOnlyScoreFailsIfInverseCollectionHasExtraElements(SolutionManagerSource SolutionManagerSource) { + var solution = new TestdataInverseRelationSolution("solution"); + var e1 = new TestdataInverseRelationEntity("e1"); + var e2 = new TestdataInverseRelationEntity("e2"); + var e3 = new TestdataInverseRelationEntity("e3"); + + var v1 = new TestdataInverseRelationValue("v1"); + var v2 = new TestdataInverseRelationValue("v2"); + + v1.setEntities(List.of(e1, e2)); + v2.setEntities(List.of(e3, e1)); + + solution.setEntityList(List.of(e1, e2, e3)); + solution.setValueList(List.of(v1, v2)); + + e1.setValue(v1); + e2.setValue(v1); + e3.setValue(v2); + + assertSoftly(softly -> { + softly.assertThat(solution.getScore()).isNull(); + softly.assertThat(solution.getValueList().get(0).getEntities()).containsExactly(e1, e2); + softly.assertThat(solution.getValueList().get(1).getEntities()).containsExactly(e3, e1); + }); + + var solutionManager = SolutionManagerSource.createSolutionManager(SOLVER_FACTORY_INVERSE_RELATION); + assertThat(solutionManager).isNotNull(); + assertThatCode(() -> { + solutionManager.update(solution, SolutionUpdatePolicy.UPDATE_SCORE_ONLY); + }).hasMessageContainingAll( + "The entity (v2)", + "has a collection @" + InverseRelationShadowVariable.class.getSimpleName(), + "([e3, e1])", + "that points to a source entity (e1)", + "but that source entity points to (v1) instead"); + } + + @ParameterizedTest + @EnumSource(SolutionManagerSource.class) + void updateOnlyScoreFailsIfListVariableInconsistent(SolutionManagerSource SolutionManagerSource) { + var solution = new TestdataConcurrentSolution(); + var e1 = new TestdataConcurrentEntity("e1"); + var e2 = new TestdataConcurrentEntity("e2"); + + var a1 = new TestdataConcurrentValue("a1"); + var a2 = new TestdataConcurrentValue("a2"); + var b1 = new TestdataConcurrentValue("b1"); + var b2 = new TestdataConcurrentValue("b2"); + + var groupA = List.of(a1, a2); + var groupB = List.of(b1, b2); + + var entities = List.of(e1, e2); + var values = List.of(a1, a2, b1, b2); + + a1.setConcurrentValueGroup(groupA); + a2.setConcurrentValueGroup(groupA); + + b1.setConcurrentValueGroup(groupB); + b2.setConcurrentValueGroup(groupB); + + e1.setValues(List.of(a1, b1)); + e2.setValues(List.of(b2, a2)); + + b1.setPreviousValue(a1); + a2.setPreviousValue(b2); + + a1.setNextValue(b1); + b2.setNextValue(a2); + + a1.setIndex(0); + a2.setIndex(1); + + b1.setIndex(1); + b2.setIndex(0); + + a1.setEntity(e1); + b1.setEntity(e2); + a2.setEntity(e2); + b2.setEntity(e2); + + a1.setInconsistent(true); + a2.setInconsistent(true); + b1.setInconsistent(true); + b2.setInconsistent(true); + + solution.setEntities(entities); + solution.setValues(values); + + var solutionManager = SolutionManagerSource.createSolutionManager(SOLVER_FACTORY_DECLARATIVE_SHADOW); + assertThat(solutionManager).isNotNull(); + assertThatCode(() -> { + solutionManager.update(solution, SolutionUpdatePolicy.UPDATE_SCORE_ONLY); + }).hasMessageContainingAll( + "The entity (e1)" + + " has a list variable (values)" + + " and one of its elements (e1 -> a1 -> b1)" + + " which has a shadow variable (entity)" + + " has an oldInverseEntity (e2) which is not that entity."); + } + @ParameterizedTest @EnumSource(SolutionManagerSource.class) void explain(SolutionManagerSource SolutionManagerSource) { diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java index 260dfd0c73..8aaa466ed1 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java @@ -20,7 +20,7 @@ import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningSolutionMetaModel; import ai.timefold.solver.core.preview.api.domain.metamodel.ShadowVariableMetaModel; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; import ai.timefold.solver.core.testdomain.TestdataEntity; import ai.timefold.solver.core.testdomain.TestdataObject; import ai.timefold.solver.core.testdomain.TestdataSolution; @@ -696,23 +696,24 @@ record RootClass(TestClass inner) { } @Test - void errorIfShadowVariableLoopedReferenced() { + void errorIfShadowVariablesInconsistentReferenced() { assertThatCode(() -> RootVariableSource.from( planningSolutionMetaModel, TestdataInvalidDeclarativeValue.class, "shadow", - "isLooped", + "isInconsistent", DEFAULT_MEMBER_ACCESSOR_FACTORY, DEFAULT_DESCRIPTOR_POLICY)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContainingAll( - "The source path (isLooped) starting from root class (%s) accesses a @%s property (isLooped)" + "The source path (isInconsistent) starting from root class (%s) accesses a @%s property (isInconsistent)" .formatted(TestdataInvalidDeclarativeValue.class.getCanonicalName(), - ShadowVariableLooped.class.getSimpleName()), - "Supplier methods are only called when none of their dependencies are looped", + ShadowVariablesInconsistent.class.getSimpleName()), + "Supplier methods are only called when none of their dependencies are inconsistent", "reading @%s properties are not needed since they are guaranteed to be false" - .formatted(ShadowVariableLooped.class.getSimpleName()), - "Maybe remove the source path (isLooped) from the @%s?".formatted(ShadowSources.class.getSimpleName())); + .formatted(ShadowVariablesInconsistent.class.getSimpleName()), + "Maybe remove the source path (isInconsistent) from the @%s?" + .formatted(ShadowSources.class.getSimpleName())); } @Test diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index de153bfef3..1dfb329987 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -102,6 +102,10 @@ import ai.timefold.solver.core.testdomain.chained.multientity.TestdataChainedGreenEntity; import ai.timefold.solver.core.testdomain.chained.multientity.TestdataChainedMultiEntityAnchor; import ai.timefold.solver.core.testdomain.chained.multientity.TestdataChainedMultiEntitySolution; +import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentConstraintProvider; +import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentEntity; +import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentSolution; +import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentValue; import ai.timefold.solver.core.testdomain.list.TestdataListEntity; import ai.timefold.solver.core.testdomain.list.TestdataListSolution; import ai.timefold.solver.core.testdomain.list.TestdataListValue; @@ -141,6 +145,10 @@ import ai.timefold.solver.core.testdomain.pinned.TestdataPinnedEntity; import ai.timefold.solver.core.testdomain.pinned.TestdataPinnedSolution; import ai.timefold.solver.core.testdomain.score.TestdataHardSoftScoreSolution; +import ai.timefold.solver.core.testdomain.shadow.inverserelation.TestdataInverseRelationConstraintProvider; +import ai.timefold.solver.core.testdomain.shadow.inverserelation.TestdataInverseRelationEntity; +import ai.timefold.solver.core.testdomain.shadow.inverserelation.TestdataInverseRelationSolution; +import ai.timefold.solver.core.testdomain.shadow.inverserelation.TestdataInverseRelationValue; import ai.timefold.solver.core.testdomain.valuerange.entityproviding.unassignedvar.TestdataAllowsUnassignedEntityProvidingEntity; import ai.timefold.solver.core.testdomain.valuerange.entityproviding.unassignedvar.TestdataAllowsUnassignedEntityProvidingScoreCalculator; import ai.timefold.solver.core.testdomain.valuerange.entityproviding.unassignedvar.TestdataAllowsUnassignedEntityProvidingSolution; @@ -1970,6 +1978,77 @@ void solvePinnedAndUnassignedMixedModel() { assertThat(solution.getEntityList().get(1).getValueList()).hasSameElementsAs(List.of(problem.getValueList().get(3))); } + @Test + void solveStaleBuiltinShadows() { + // Solver config + var solverConfig = PlannerTestUtils.buildSolverConfig( + TestdataInverseRelationSolution.class, TestdataInverseRelationEntity.class, TestdataInverseRelationValue.class) + .withEasyScoreCalculatorClass(null) + .withConstraintProviderClass(TestdataInverseRelationConstraintProvider.class); + + var problem = new TestdataInverseRelationSolution(); + var e1 = new TestdataInverseRelationEntity("e1"); + var e2 = new TestdataInverseRelationEntity("e2"); + + var v1 = new TestdataInverseRelationValue("v1"); + var v2 = new TestdataInverseRelationValue("v2"); + + e1.setValue(v1); + e2.setValue(v1); + + problem.setEntityList(List.of(e1, e2)); + problem.setValueList(List.of(v1, v2)); + + var solution = PlannerTestUtils.solve(solverConfig, problem); + + assertThat(solution.getEntityList().get(0).getValue().getCode()).isEqualTo("v1"); + assertThat(solution.getEntityList().get(1).getValue().getCode()).isEqualTo("v2"); + + assertThat(solution.getScore()).isEqualTo(SimpleScore.of(-2)); + } + + @Test + void solveStaleDeclarativeShadows() { + // Solver config + var solverConfig = PlannerTestUtils.buildSolverConfig( + TestdataConcurrentSolution.class, TestdataConcurrentEntity.class, TestdataConcurrentValue.class) + .withPreviewFeature(DECLARATIVE_SHADOW_VARIABLES) + .withEasyScoreCalculatorClass(null) + .withConstraintProviderClass(TestdataConcurrentConstraintProvider.class); + + var e1 = new TestdataConcurrentEntity("e1"); + var e2 = new TestdataConcurrentEntity("e2"); + + var a1 = new TestdataConcurrentValue("a1"); + var a2 = new TestdataConcurrentValue("a2"); + var b1 = new TestdataConcurrentValue("b1"); + var b2 = new TestdataConcurrentValue("b2"); + + a1.setConcurrentValueGroup(List.of(a1, a2)); + a2.setConcurrentValueGroup(List.of(a1, a2)); + + b1.setConcurrentValueGroup(List.of(b1, b2)); + b2.setConcurrentValueGroup(List.of(b1, b2)); + + e1.setValues(List.of(a1, b1)); + e2.setValues(List.of(b2, a2)); + + var entities = List.of(e1, e2); + var values = List.of(a1, a2, b1, b2); + + var problem = new TestdataConcurrentSolution(); + + problem.setEntities(entities); + problem.setValues(values); + + var solution = PlannerTestUtils.solve(solverConfig, problem); + + assertThat(solution.getEntities().get(0).getValues()).map(TestdataConcurrentValue::getId).containsExactly("a1", "b1"); + assertThat(solution.getEntities().get(1).getValues()).map(TestdataConcurrentValue::getId).containsExactly("a2", "b2"); + + assertThat(solution.getScore()).isEqualTo(HardSoftScore.of(0, -240)); + } + private static List generateMovesForMixedModel() { // Local Search var allMoveSelectionConfigList = new ArrayList(); diff --git a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/concurrent/ConcurrentValuesShadowVariableTest.java b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/concurrent/ConcurrentValuesShadowVariableTest.java index 7e7a870996..a86e2713b1 100644 --- a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/concurrent/ConcurrentValuesShadowVariableTest.java +++ b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/concurrent/ConcurrentValuesShadowVariableTest.java @@ -224,9 +224,9 @@ void groupChainValidToInvalid() { SolutionManager.updateShadowVariables(TestdataConcurrentSolution.class, entity1, entity2, entity3, valueA1, valueA2, valueB1, valueB2, valueB3, valueC); - // Everything is invalid/null, since no values are prior to the looped + // Everything is invalid/null, since no values are prior to the inconsistent // groups. - // C is invalid, since it is after the concurrent loop + // C is inconsistent, since it is after the concurrent loop assertInvalid(valueA1, valueA2, valueB1, valueB2, valueB3, valueC); // Third test: @@ -309,7 +309,7 @@ private static void assertStartsAfterDuration(Duration duration, TestdataConcurr for (var value : values) { assertThat(value.getServiceStartTime()).isEqualTo(BASE_START_TIME.plus(duration)); assertThat(value.getServiceFinishTime()).isEqualTo(BASE_START_TIME.plus(duration).plusMinutes(30L)); - assertThat(value.isInvalid()).isFalse(); + assertThat(value.isInconsistent()).isFalse(); } } @@ -317,7 +317,7 @@ private static void assertInvalid(TestdataConcurrentValue... values) { for (var value : values) { assertThat(value.getServiceStartTime()).isNull(); assertThat(value.getServiceFinishTime()).isNull(); - assertThat(value.isInvalid()).isTrue(); + assertThat(value.isInconsistent()).isTrue(); } } diff --git a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/dependent/DependencyValuesShadowVariableTest.java b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/dependent/DependencyValuesShadowVariableTest.java index 3392118c61..4af048f837 100644 --- a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/dependent/DependencyValuesShadowVariableTest.java +++ b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/dependent/DependencyValuesShadowVariableTest.java @@ -99,9 +99,9 @@ void testLoopStatusOfEntityIsUpdatedEvenIfNoVariablesOnTheEntityChanged() { // Tests the move [A, B] -> [C, A, B]. // Since C depends on A and B, this is an invalid solution, - // and C.startTime/C.endTime remains null and C.isLooped is true. + // and C.startTime/C.endTime remains null and C.isInconsistent is true. // When the move is undone, C.startTime/C.endTime remains null, - // and C.isLooped is false. + // and C.isInconsistent is false. moveAsserter.assertMoveAndUndo(schedule, new ListAssignMove<>( (PlanningListVariableMetaModel) solutionDescriptor .getListVariableDescriptor().getVariableMetaModel(), diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/basic/TestdataBasicVarValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/basic/TestdataBasicVarValue.java index ba8a8893eb..c22c2e2d0d 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/basic/TestdataBasicVarValue.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/basic/TestdataBasicVarValue.java @@ -9,7 +9,7 @@ import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; @PlanningEntity public class TestdataBasicVarValue { @@ -24,7 +24,7 @@ public class TestdataBasicVarValue { @ShadowVariable(supplierName = "calculateEndTime") LocalDateTime endTime; - @ShadowVariableLooped + @ShadowVariablesInconsistent boolean isInvalid; @InverseRelationShadowVariable(sourceVariableName = "value") diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/chained/TestdataChainedVarValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/chained/TestdataChainedVarValue.java index 77d0d65575..469e808433 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/chained/TestdataChainedVarValue.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/chained/TestdataChainedVarValue.java @@ -7,7 +7,7 @@ import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; @PlanningEntity public class TestdataChainedVarValue { @@ -22,7 +22,7 @@ public class TestdataChainedVarValue { @ShadowVariable(supplierName = "calculateEndTime") LocalDateTime endTime; - @ShadowVariableLooped + @ShadowVariablesInconsistent boolean isInvalid; @InverseRelationShadowVariable(sourceVariableName = "previous") diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentAssertionConstraintProvider.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentAssertionConstraintProvider.java index a5bb639d80..f70415f8d0 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentAssertionConstraintProvider.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentAssertionConstraintProvider.java @@ -16,12 +16,12 @@ public class TestdataConcurrentAssertionConstraintProvider implements Constraint public Constraint @NonNull [] defineConstraints(@NonNull ConstraintFactory constraintFactory) { return new Constraint[] { constraintFactory.forEach(TestdataConcurrentValue.class) - .filter(TestdataConcurrentValue::getExpectedInvalid) + .filter(TestdataConcurrentValue::getExpectedInconsistent) .penalize(HardSoftScore.ONE_HARD) .asConstraint("Invalid visit"), constraintFactory.forEach(TestdataConcurrentValue.class) - .filter(visit -> !visit.getExpectedInvalid() && visit.isAssigned()) + .filter(visit -> !visit.getExpectedInconsistent() && visit.isAssigned()) .penalize(HardSoftScore.ONE_SOFT, visit -> (int) Duration .between(BASE_START_TIME, visit.getExpectedServiceFinishTime()) diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentAssertionEasyScoreCalculator.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentAssertionEasyScoreCalculator.java index c41371b22c..1121ded107 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentAssertionEasyScoreCalculator.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentAssertionEasyScoreCalculator.java @@ -18,7 +18,7 @@ public class TestdataConcurrentAssertionEasyScoreCalculator for (var visit : routePlan.values) { if (visit.isAssigned()) { - if (visit.getExpectedInvalid()) { + if (visit.getExpectedInconsistent()) { hardScore--; } else { softScore -= (int) Duration diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentConstraintProvider.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentConstraintProvider.java index 3fc3d28767..3ad84471e4 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentConstraintProvider.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentConstraintProvider.java @@ -16,12 +16,12 @@ public class TestdataConcurrentConstraintProvider implements ConstraintProvider public Constraint @NonNull [] defineConstraints(@NonNull ConstraintFactory constraintFactory) { return new Constraint[] { constraintFactory.forEach(TestdataConcurrentValue.class) - .filter(TestdataConcurrentValue::isInvalid) + .filter(TestdataConcurrentValue::isInconsistent) .penalize(HardSoftScore.ONE_HARD) .asConstraint("Invalid visit"), constraintFactory.forEach(TestdataConcurrentValue.class) - .filter(visit -> !visit.isInvalid() && visit.isAssigned()) + .filter(visit -> !visit.isInconsistent() && visit.isAssigned()) .penalize(HardSoftScore.ONE_SOFT, visit -> (int) Duration .between(BASE_START_TIME, visit.getServiceFinishTime()) .toMinutes()) diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentEasyScoreCalculator.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentEasyScoreCalculator.java index 2273f1803c..5a4ff9ec71 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentEasyScoreCalculator.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentEasyScoreCalculator.java @@ -17,7 +17,7 @@ public class TestdataConcurrentEasyScoreCalculator implements EasyScoreCalculato for (var visit : routePlan.values) { if (visit.isAssigned()) { - if (visit.isInvalid()) { + if (visit.isInconsistent()) { hardScore--; } else { softScore -= (int) Duration diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentValue.java index f1fc86551c..e3779a7d66 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentValue.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/concurrent/TestdataConcurrentValue.java @@ -17,7 +17,7 @@ import ai.timefold.solver.core.api.domain.variable.PreviousElementShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; @PlanningEntity public class TestdataConcurrentValue { @@ -50,8 +50,8 @@ public class TestdataConcurrentValue { List concurrentValueGroup; - @ShadowVariableLooped - boolean isInvalid; + @ShadowVariablesInconsistent + Boolean isInconsistent; public TestdataConcurrentValue() { } @@ -131,8 +131,9 @@ public LocalDateTime serviceReadyTimeUpdater() { value = { "serviceReadyTime", "concurrentValueGroup[].serviceReadyTime" }, alignmentKey = "concurrentValueGroup") public LocalDateTime serviceStartTimeUpdater() { - if (isInvalid) { - throw new IllegalStateException("Value (%s) is looped when serviceStartTimeUpdater is called.".formatted(this)); + if (isInconsistent) { + throw new IllegalStateException( + "Value (%s) is inconsistent when serviceStartTimeUpdater is called.".formatted(this)); } if (serviceReadyTime == null) { return null; @@ -140,9 +141,9 @@ public LocalDateTime serviceStartTimeUpdater() { var startTime = serviceReadyTime; if (concurrentValueGroup != null) { for (var visit : concurrentValueGroup) { - if (visit.isInvalid) { + if (visit.isInconsistent) { throw new IllegalStateException( - "Value (%s) is looped when serviceStartTimeUpdater is called.".formatted(visit)); + "Value (%s) is inconsistent when serviceStartTimeUpdater is called.".formatted(visit)); } if (visit.serviceReadyTime != null && startTime.isBefore(visit.serviceReadyTime)) { startTime = visit.serviceReadyTime; @@ -184,24 +185,24 @@ public void setConcurrentValueGroup(List concurrentValu this.concurrentValueGroup = concurrentValueGroup; } - public boolean isInvalid() { - return isInvalid; + public Boolean isInconsistent() { + return isInconsistent; } - public void setInvalid(boolean invalid) { - isInvalid = invalid; + public void setInconsistent(Boolean inconsistent) { + isInconsistent = inconsistent; } - public boolean getExpectedInvalid() { - return getExpectedInvalid(new IdentityHashMap<>()); + public boolean getExpectedInconsistent() { + return getExpectedInconsistent(new IdentityHashMap<>()); } - boolean getExpectedInvalid(Map cache) { + boolean getExpectedInconsistent(Map cache) { if (cache.containsKey(this)) { return cache.get(this); } cache.put(this, true); - if (previousValue != null && previousValue.getExpectedInvalid(cache)) { + if (previousValue != null && previousValue.getExpectedInconsistent(cache)) { return true; } if (concurrentValueGroup != null) { @@ -210,7 +211,7 @@ boolean getExpectedInvalid(Map cache) { if (visit.entity != null && !vehicles.add(visit.entity)) { return true; } - if (visit != this && visit.previousValue != null && visit.previousValue.getExpectedInvalid(cache)) { + if (visit != this && visit.previousValue != null && visit.previousValue.getExpectedInconsistent(cache)) { return true; } } @@ -232,7 +233,7 @@ public LocalDateTime getExpectedServiceReadyTime() { } public LocalDateTime getExpectedServiceReadyTime(TimeCache cache) { - if (getExpectedInvalid()) { + if (getExpectedInconsistent()) { return null; } if (cache.readyTimeCache.containsKey(this)) { @@ -260,7 +261,7 @@ public LocalDateTime getExpectedServiceStartTime() { } public LocalDateTime getExpectedServiceStartTime(TimeCache cache) { - if (getExpectedInvalid()) { + if (getExpectedInconsistent()) { return null; } if (cache.startTimeCache.containsKey(this)) { @@ -285,7 +286,7 @@ public LocalDateTime getExpectedServiceFinishTime() { } public LocalDateTime getExpectedServiceFinishTime(TimeCache cache) { - if (getExpectedInvalid()) { + if (getExpectedInconsistent()) { return null; } if (cache.endTimeCache.containsKey(this)) { diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/dependency/TestdataDependencyValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/dependency/TestdataDependencyValue.java index a7d83ec7f6..da3d0abd7e 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/dependency/TestdataDependencyValue.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/dependency/TestdataDependencyValue.java @@ -9,7 +9,7 @@ import ai.timefold.solver.core.api.domain.variable.PreviousElementShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; import org.apache.commons.lang3.ObjectUtils; @@ -27,7 +27,7 @@ public class TestdataDependencyValue { @ShadowVariable(supplierName = "calculateEndTime") LocalDateTime endTime; - @ShadowVariableLooped + @ShadowVariablesInconsistent boolean isInvalid; @InverseRelationShadowVariable(sourceVariableName = "values") diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/invalid/TestdataInvalidDeclarativeValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/invalid/TestdataInvalidDeclarativeValue.java index 6d94fe02fe..90a8942d73 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/invalid/TestdataInvalidDeclarativeValue.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/invalid/TestdataInvalidDeclarativeValue.java @@ -6,7 +6,7 @@ import ai.timefold.solver.core.api.domain.variable.PreviousElementShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; import ai.timefold.solver.core.testdomain.TestdataObject; @PlanningEntity @@ -24,8 +24,8 @@ public class TestdataInvalidDeclarativeValue extends TestdataObject { @ShadowVariable(supplierName = "shadowSupplier") TestdataInvalidDeclarativeValue shadow; - @ShadowVariableLooped - boolean isLooped; + @ShadowVariablesInconsistent + boolean isInconsistent; public TestdataInvalidDeclarativeValue() { } @@ -74,12 +74,12 @@ public void setShadow(TestdataInvalidDeclarativeValue shadow) { this.shadow = shadow; } - public boolean isLooped() { - return isLooped; + public boolean isInconsistent() { + return isInconsistent; } - public void setLooped(boolean looped) { - isLooped = looped; + public void setInconsistent(boolean inconsistent) { + isInconsistent = inconsistent; } @ShadowSources("previous") diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/missing/TestdataDeclarativeMissingSupplierValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/missing/TestdataDeclarativeMissingSupplierValue.java index 6ad76998ef..18f2903bd6 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/missing/TestdataDeclarativeMissingSupplierValue.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/missing/TestdataDeclarativeMissingSupplierValue.java @@ -9,7 +9,7 @@ import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; @PlanningEntity public class TestdataDeclarativeMissingSupplierValue { @@ -24,7 +24,7 @@ public class TestdataDeclarativeMissingSupplierValue { @ShadowVariable(supplierName = "calculateEndTime") LocalDateTime endTime; - @ShadowVariableLooped + @ShadowVariablesInconsistent boolean isInvalid; @InverseRelationShadowVariable(sourceVariableName = "value") diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentValue.java index 32ddc8e252..1342b52b95 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentValue.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentValue.java @@ -12,7 +12,7 @@ import ai.timefold.solver.core.api.domain.variable.PreviousElementShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; @PlanningEntity public class TestdataMultiDirectionConcurrentValue { @@ -42,7 +42,7 @@ public class TestdataMultiDirectionConcurrentValue { List concurrentValueGroup; - @ShadowVariableLooped + @ShadowVariablesInconsistent boolean isInvalid; public TestdataMultiDirectionConcurrentValue() { diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyValue.java index 702e98c825..483f450431 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyValue.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyValue.java @@ -9,7 +9,7 @@ import ai.timefold.solver.core.api.domain.variable.PreviousElementShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; import org.apache.commons.lang3.ObjectUtils; @@ -27,7 +27,7 @@ public class TestdataMultiEntityDependencyValue { @ShadowVariable(supplierName = "calculateEndTime") LocalDateTime endTime; - @ShadowVariableLooped + @ShadowVariablesInconsistent boolean isInvalid; @InverseRelationShadowVariable(sourceVariableName = "values") diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/shadow/inverserelation/TestdataInverseRelationConstraintProvider.java b/core/src/test/java/ai/timefold/solver/core/testdomain/shadow/inverserelation/TestdataInverseRelationConstraintProvider.java new file mode 100644 index 0000000000..5a5e6c4868 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/shadow/inverserelation/TestdataInverseRelationConstraintProvider.java @@ -0,0 +1,23 @@ +package ai.timefold.solver.core.testdomain.shadow.inverserelation; + +import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore; +import ai.timefold.solver.core.api.score.stream.Constraint; +import ai.timefold.solver.core.api.score.stream.ConstraintFactory; +import ai.timefold.solver.core.api.score.stream.ConstraintProvider; + +import org.jspecify.annotations.NonNull; + +public class TestdataInverseRelationConstraintProvider implements ConstraintProvider { + @Override + public Constraint @NonNull [] defineConstraints(@NonNull ConstraintFactory constraintFactory) { + return new Constraint[] { + constraintFactory.forEach(TestdataInverseRelationValue.class) + .penalize(SimpleScore.ONE, value -> value.getEntities().size() * value.getEntities().size()) + .asConstraint("Balance values"), + constraintFactory.forEachIncludingUnassigned(TestdataInverseRelationEntity.class) + .filter(entity -> entity.getValue() == null) + .penalize(SimpleScore.of(100)) + .asConstraint("Unassigned entity") + }; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/shadow/inverserelation/TestdataInverseRelationEasyScoreCalculator.java b/core/src/test/java/ai/timefold/solver/core/testdomain/shadow/inverserelation/TestdataInverseRelationEasyScoreCalculator.java new file mode 100644 index 0000000000..0c1d621955 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/shadow/inverserelation/TestdataInverseRelationEasyScoreCalculator.java @@ -0,0 +1,18 @@ +package ai.timefold.solver.core.testdomain.shadow.inverserelation; + +import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore; +import ai.timefold.solver.core.api.score.calculator.EasyScoreCalculator; + +import org.jspecify.annotations.NonNull; + +public class TestdataInverseRelationEasyScoreCalculator + implements EasyScoreCalculator { + @Override + public @NonNull SimpleScore calculateScore(@NonNull TestdataInverseRelationSolution testdataInverseRelationSolution) { + int score = 0; + for (var value : testdataInverseRelationSolution.getValueList()) { + score -= value.getEntities().size() * value.getEntities().size(); + } + return SimpleScore.of(score); + } +} diff --git a/docs/src/modules/ROOT/pages/using-timefold-solver/modeling-planning-problems.adoc b/docs/src/modules/ROOT/pages/using-timefold-solver/modeling-planning-problems.adoc index 483011a34a..adf506042d 100644 --- a/docs/src/modules/ROOT/pages/using-timefold-solver/modeling-planning-problems.adoc +++ b/docs/src/modules/ROOT/pages/using-timefold-solver/modeling-planning-problems.adoc @@ -956,21 +956,20 @@ public class Job { |Accesses `endTime` of each element in the `dependencies` collection. |=== - -==== Detecting Loops in Shadow Variables +==== Detecting Inconsistencies in Shadow Variables In certain cases, shadow variables may form an infinite loop. When this occurs, the solver detects the cycle, breaks the loop, and assigns `null` to the involved shadow variables. -A declarative shadow variable is considered part of a _loop_ if: +A declarative shadow variable is considered _inconsistent_ if: - It directly or indirectly depends on itself. For example, if variable `a` depends on `b` and `b` depends on `a`, both are in a loop. -- It depends on another variable that is already part of a loop. +- It depends on another variable that is inconsistent. For example, if `c` depends on `a`, and `a` is in a loop with `b`, then `c` is also considered part of the loop. -To detect whether an entity has been affected by a shadow variable loop, annotate a boolean field with `@ShadowVariableLooped`. -The solver will set this field to true if the entity is part of a loop. +To detect whether an entity has inconsistent shadow variables, annotate a boolean field with `@ShadowVariablesInconsistent`. +The solver will set this field to true if the entity has any inconsistent shadow variables. [tabs] ==== @@ -980,18 +979,17 @@ Java:: ---- @PlanningEntity public class Job { - @ShadowVariableLooped - boolean looped; + @ShadowVariablesInconsistent + boolean isInconsistent; - public boolean isLooped() { - return looped; + public boolean isInconsistent() { + return isInconsistent; } } ---- ==== -This property (`looped` in the example above) is typically used in a constraint filter to penalize looped entities via a hard constraint, -since PlanningSolution instances containing loops are generally considered invalid. +This property (`isInconsistent` in the example above) is typically used in a constraint filter to penalize entities with inconsistent shadow variables via a hard constraint, since PlanningSolution instances containing inconsistent shadow variables are generally considered invalid. [tabs] ==== @@ -1000,19 +998,19 @@ Java:: [source,java,options="nowrap"] ---- //Example constraint -Constraint penalizeLoopedJobs(ConstraintFactory factory) { +Constraint penalizeInconsistentJobs(ConstraintFactory factory) { return factory.forEach(Job.class) - .filter(job -> job.isLooped()) + .filter(job -> job.isInconsistent()) .penalize(HardSoftScore.ONE_HARD) - .asConstraint("Job has looped shadow variables."); + .asConstraint("Job has inconsistent shadow variables."); } ---- ==== -NOTE: `@ShadowSources`-annotated methods do not need to check `@ShadowVariableLooped`-annotated properties. -These methods are only called if neither of their dependencies are looped, and therefore the value of such properties at that time is guaranteed to be `false`. +NOTE: `@ShadowSources`-annotated methods do not need to check `@ShadowVariablesInconsistent`-annotated properties. +These methods are only called if none of their shadow variables are inconsistent, and therefore the value of such properties at that time is guaranteed to be `false`. [#aligningShadowVariables] ==== Aligning Shadow Variables diff --git a/quarkus-integration/quarkus/deployment/src/main/java/ai/timefold/solver/quarkus/deployment/DotNames.java b/quarkus-integration/quarkus/deployment/src/main/java/ai/timefold/solver/quarkus/deployment/DotNames.java index 68bb420c41..fd69ee300f 100644 --- a/quarkus-integration/quarkus/deployment/src/main/java/ai/timefold/solver/quarkus/deployment/DotNames.java +++ b/quarkus-integration/quarkus/deployment/src/main/java/ai/timefold/solver/quarkus/deployment/DotNames.java @@ -40,7 +40,7 @@ import ai.timefold.solver.core.config.solver.SolverConfig; import ai.timefold.solver.core.config.solver.SolverManagerConfig; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; import org.jboss.jandex.DotName; @@ -84,7 +84,7 @@ public final class DotNames { static final DotName PIGGYBACK_SHADOW_VARIABLE = DotName.createSimple(PiggybackShadowVariable.class.getName()); static final DotName PREVIOUS_ELEMENT_SHADOW_VARIABLE = DotName.createSimple(PreviousElementShadowVariable.class.getName()); static final DotName SHADOW_VARIABLE = DotName.createSimple(ShadowVariable.class.getName()); - static final DotName SHADOW_VARIABLE_LOOPED = DotName.createSimple(ShadowVariableLooped.class.getName()); + static final DotName SHADOW_VARIABLES_INCONSISTENT = DotName.createSimple(ShadowVariablesInconsistent.class.getName()); static final DotName CASCADING_UPDATE_SHADOW_VARIABLE = DotName.createSimple(CascadingUpdateShadowVariable.class.getName()); static final DotName SHADOW_SOURCES = DotName.createSimple(ShadowSources.class.getName()); @@ -111,7 +111,7 @@ public final class DotNames { PIGGYBACK_SHADOW_VARIABLE, PREVIOUS_ELEMENT_SHADOW_VARIABLE, SHADOW_VARIABLE, - SHADOW_VARIABLE_LOOPED, + SHADOW_VARIABLES_INCONSISTENT, CASCADING_UPDATE_SHADOW_VARIABLE }; @@ -138,7 +138,7 @@ public final class DotNames { PIGGYBACK_SHADOW_VARIABLE, PREVIOUS_ELEMENT_SHADOW_VARIABLE, SHADOW_VARIABLE, - SHADOW_VARIABLE_LOOPED, + SHADOW_VARIABLES_INCONSISTENT, CASCADING_UPDATE_SHADOW_VARIABLE }; diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/gizmo/TestDataKitchenSinkEntity.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/gizmo/TestDataKitchenSinkEntity.java index 2433920f95..e18f40650b 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/gizmo/TestDataKitchenSinkEntity.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/gizmo/TestDataKitchenSinkEntity.java @@ -12,7 +12,7 @@ import ai.timefold.solver.core.api.domain.variable.PlanningVariableReference; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; /* * Should have one of every annotation, even annotations that @@ -41,8 +41,8 @@ public class TestDataKitchenSinkEntity { @ShadowVariable(supplierName = "copyStringVariable") private String declarativeShadowVariable; - @ShadowVariableLooped - private boolean shadowVariableLooped; + @ShadowVariablesInconsistent + private boolean isInconsistent; @PiggybackShadowVariable(shadowVariableName = "shadow2") private String piggybackShadow; diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/list/TestdataQuarkusSupplierVariableListConstraintProvider.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/list/TestdataQuarkusSupplierVariableListConstraintProvider.java index de31b67c3a..9fb200d370 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/list/TestdataQuarkusSupplierVariableListConstraintProvider.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/list/TestdataQuarkusSupplierVariableListConstraintProvider.java @@ -13,13 +13,13 @@ public class TestdataQuarkusSupplierVariableListConstraintProvider implements Co public Constraint @NonNull [] defineConstraints(@NonNull ConstraintFactory factory) { return new Constraint[] { factory.forEach(TestdataQuarkusSupplierVariableListValue.class) - .filter(value -> !value.isLooped()) + .filter(value -> !value.isInconsistent()) .penalize(SimpleScore.ONE, TestdataQuarkusSupplierVariableListValue::getStartTime) .asConstraint("Minimize start time"), factory.forEach(TestdataQuarkusSupplierVariableListValue.class) - .filter(TestdataQuarkusSupplierVariableListValue::isLooped) + .filter(TestdataQuarkusSupplierVariableListValue::isInconsistent) .penalize(SimpleScore.of(1000)) - .asConstraint("Looped") + .asConstraint("Inconsistent") }; } diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/list/TestdataQuarkusSupplierVariableListValue.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/list/TestdataQuarkusSupplierVariableListValue.java index 9068b1ddc6..cb38a357a7 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/list/TestdataQuarkusSupplierVariableListValue.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/list/TestdataQuarkusSupplierVariableListValue.java @@ -6,7 +6,7 @@ import ai.timefold.solver.core.api.domain.variable.PreviousElementShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; @PlanningEntity public class TestdataQuarkusSupplierVariableListValue { @@ -20,8 +20,8 @@ public class TestdataQuarkusSupplierVariableListValue { @ShadowVariable(supplierName = "startTimeSupplier") private Integer startTime; - @ShadowVariableLooped - private boolean looped; + @ShadowVariablesInconsistent + private boolean isInconsistent; public TestdataQuarkusSupplierVariableListValue() { } @@ -43,12 +43,12 @@ public void setStartTime(Integer startTime) { this.startTime = startTime; } - public boolean isLooped() { - return looped; + public boolean isInconsistent() { + return isInconsistent; } - public void setLooped(boolean looped) { - this.looped = looped; + public void setInconsistent(boolean inconsistent) { + this.isInconsistent = inconsistent; } @ShadowSources({ "previous.startTime", "dependencies[].startTime" }) diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/missing/TestdataQuarkusDeclarativeMissingSupplierValue.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/missing/TestdataQuarkusDeclarativeMissingSupplierValue.java index de789bed10..14df555022 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/missing/TestdataQuarkusDeclarativeMissingSupplierValue.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/suppliervariable/missing/TestdataQuarkusDeclarativeMissingSupplierValue.java @@ -9,7 +9,7 @@ import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariableLooped; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; @PlanningEntity public class TestdataQuarkusDeclarativeMissingSupplierValue { @@ -24,7 +24,7 @@ public class TestdataQuarkusDeclarativeMissingSupplierValue { @ShadowVariable(supplierName = "calculateEndTime") LocalDateTime endTime; - @ShadowVariableLooped + @ShadowVariablesInconsistent boolean isInvalid; @InverseRelationShadowVariable(sourceVariableName = "value") From 1115f8837ff7c996e8ba8cc5854617eecc319491 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Tue, 19 Aug 2025 17:33:26 -0400 Subject: [PATCH 2/6] chore: review comments --- .../support/VariableListenerSupport.java | 120 ------------ .../score/director/AbstractScoreDirector.java | 26 +-- .../AbstractScoreDirectorFactory.java | 25 +-- .../impl/score/director/EntityValidator.java | 183 ++++++++++++++++++ .../score/director/InnerScoreDirector.java | 18 +- .../core/impl/solver/DefaultSolver.java | 2 +- .../core/impl/solver/scope/SolverScope.java | 13 +- .../core/api/solver/SolutionManagerTest.java | 5 +- ...DefaultConstructionHeuristicPhaseTest.java | 4 +- 9 files changed, 224 insertions(+), 172 deletions(-) create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/score/director/EntityValidator.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java index 63d98db738..a3f5870594 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java @@ -17,7 +17,6 @@ import java.util.function.IntFunction; import ai.timefold.solver.core.api.domain.solution.PlanningSolution; -import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; import ai.timefold.solver.core.api.score.director.ScoreDirector; import ai.timefold.solver.core.enterprise.TimefoldSolverEnterpriseService; import ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptor; @@ -41,7 +40,6 @@ import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager; import ai.timefold.solver.core.impl.score.director.InnerScoreDirector; import ai.timefold.solver.core.impl.util.LinkedIdentityHashSet; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; @@ -231,45 +229,6 @@ public void resetWorkingSolution() { notifiable.resetWorkingSolution(); } - if (scoreDirector.expectShadowVariablesInCorrectState()) { - var solutionDescriptor = scoreDirector.getSolutionDescriptor(); - solutionDescriptor.visitAllEntities(scoreDirector.getWorkingSolution(), - entity -> { - var entityDescriptors = solutionDescriptor.getEntityDescriptors(); - - for (var entityDescriptor : entityDescriptors) { - if (!entityDescriptor.getEntityClass().isInstance(entity)) { - continue; - } - var inconsistentShadowVariablesDescriptor = - entityDescriptor.getShadowVariablesInconsistentDescriptor(); - if (inconsistentShadowVariablesDescriptor != null - && inconsistentShadowVariablesDescriptor.getValue(entity) == null) { - throw new IllegalStateException( - """ - Shadow variables update is disabled, but the entity (%s) has a null @%s annotated field (%s). - Maybe enable shadow variable updates? - """ - .formatted(entity, ShadowVariablesInconsistent.class.getSimpleName(), - inconsistentShadowVariablesDescriptor.getVariableName())); - } - - var shadowVariableDescriptors = entityDescriptor.getShadowVariableDescriptors(); - for (var shadowVariableDescriptor : shadowVariableDescriptors) { - if (shadowVariableDescriptor instanceof InverseRelationShadowVariableDescriptor inverseDescriptor) { - if (inverseDescriptor.isSingleton()) { - assertSingletonInverseDescriptor(inverseDescriptor, scoreDirector.getWorkingSolution(), - entity); - } else { - assertCollectionInverseDescriptor(inverseDescriptor, scoreDirector.getWorkingSolution(), - entity); - } - } - } - } - }); - } - if (!scoreDirector.getSolutionDescriptor().getDeclarativeShadowVariableDescriptors().isEmpty()) { var shadowVariableSessionFactory = new DefaultShadowVariableSessionFactory<>( scoreDirector.getSolutionDescriptor(), @@ -280,85 +239,6 @@ Shadow variables update is disabled, but the entity (%s) has a null @%s annotate } } - private void assertSingletonInverseDescriptor(InverseRelationShadowVariableDescriptor inverseDescriptor, - Solution_ workingSolution, Object shadowEntity) { - var inverse = inverseDescriptor.getValue(shadowEntity); - var sourceDescriptor = inverseDescriptor.getSourceVariableDescriptorList().get(0); - if (sourceDescriptor.isListVariable()) { - return; // ExternalizedListInverseVariableProcessor asserts this in setInverseAsserted - } - if (inverse == null) { - scoreDirector.getSolutionDescriptor().visitAllEntities(workingSolution, maybeSourceEntity -> { - if (sourceDescriptor.getEntityDescriptor().getEntityClass().isInstance(maybeSourceEntity)) { - var sourceValue = sourceDescriptor.getValue(maybeSourceEntity); - if (sourceValue == shadowEntity) { - throw new IllegalStateException(""" - The entity (%s) has a @%s that is null, but there is a source entity (%s) that point to it. - Verify the consistency of your input solution. - """.formatted(shadowEntity, InverseRelationShadowVariable.class.getSimpleName(), - maybeSourceEntity)); - } - } - }); - } else { - var sourceValue = sourceDescriptor.getValue(inverse); - if (sourceValue != shadowEntity) { - throw new IllegalStateException( - """ - The entity (%s) has a @%s that points to a source entity (%s) but that source entity points to (%s) instead. - Verify the consistency of your input solution. - """ - .formatted(shadowEntity, InverseRelationShadowVariable.class.getSimpleName(), inverse, - sourceValue)); - } - } - } - - private void assertCollectionInverseDescriptor(InverseRelationShadowVariableDescriptor inverseDescriptor, - Solution_ workingSolution, Object shadowEntity) { - var inverseCollection = (Collection) inverseDescriptor.getValue(shadowEntity); - var sourceDescriptor = inverseDescriptor.getSourceVariableDescriptorList().get(0); - if (inverseCollection == null) { - throw new IllegalStateException(""" - The entity (%s) has a collection @%s that is null. - Verify the consistency of your input solution. - """.formatted(shadowEntity, InverseRelationShadowVariable.class.getSimpleName())); - } else { - for (var source : inverseCollection) { - var sourceValue = sourceDescriptor.getValue(source); - if (sourceValue != shadowEntity) { - throw new IllegalStateException( - """ - The entity (%s) has a collection @%s (%s) that points to a source entity (%s) but that source entity points to (%s) instead. - Verify the consistency of your input solution. - """ - .formatted(shadowEntity, InverseRelationShadowVariable.class.getSimpleName(), - inverseCollection, - source, sourceValue)); - } - } - - scoreDirector.getSolutionDescriptor().visitAllEntities(workingSolution, maybeSourceEntity -> { - if (sourceDescriptor.getEntityDescriptor().getEntityClass().isInstance(maybeSourceEntity)) { - if (inverseCollection.contains(maybeSourceEntity)) { - return; - } - var sourceValue = sourceDescriptor.getValue(maybeSourceEntity); - if (sourceValue == shadowEntity) { - throw new IllegalStateException( - """ - The entity (%s) has a collection @%s (%s), but it is missing a source entity that point to it (%s). - Verify the consistency of your input solution. - """ - .formatted(shadowEntity, InverseRelationShadowVariable.class.getSimpleName(), - inverseCollection, - maybeSourceEntity)); - } - } - }); - } - } - public void close() { for (var notifiable : notifiableRegistry.getAll()) { notifiable.closeVariableListener(); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java index 7bc54592be..86d48221a8 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java @@ -160,16 +160,6 @@ public boolean expectShadowVariablesInCorrectState() { return expectShadowVariablesInCorrectState; } - @Override - public void enableShadowVariablesInCorrectStateChecks() { - expectShadowVariablesInCorrectState = true; - } - - @Override - public void disableShadowVariablesInCorrectStateChecks() { - expectShadowVariablesInCorrectState = false; - } - @Override public @NonNull Solution_ getWorkingSolution() { return workingSolution; @@ -261,7 +251,7 @@ protected void setWorkingSolution(Solution_ workingSolution, Consumer en if (entityAndFactVisitor != null) { solutionDescriptor.visitAllProblemFacts(workingSolution, entityAndFactVisitor); } - Consumer entityValidator = entity -> scoreDirectorFactory.validateEntity(this, entity); + Consumer entityValidator = scoreDirectorFactory.getEntityValidator(this); entityAndFactVisitor = entityAndFactVisitor == null ? entityValidator : entityAndFactVisitor.andThen(entityValidator); setWorkingEntityListDirty(workingSolution); // This visits all the entities. @@ -276,6 +266,20 @@ protected void setWorkingSolution(Solution_ workingSolution, Consumer en } } + /** + * Note: Initial Solution may have stale shadow variables! + * + * @param initialSolution the initial solution + */ + @Override + public void setInitialSolution(Solution_ initialSolution) { + var originalShouldAssert = expectShadowVariablesInCorrectState; + expectShadowVariablesInCorrectState = false; + setWorkingSolution(initialSolution); + forceTriggerVariableListeners(); + expectShadowVariablesInCorrectState = originalShouldAssert; + } + @Override public void setMoveRepository(@Nullable MoveRepository moveRepository) { this.moveRepository = moveRepository; 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 f2b9bc7d15..2c8e862a50 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 @@ -1,11 +1,11 @@ package ai.timefold.solver.core.impl.score.director; +import java.util.function.Consumer; + import ai.timefold.solver.core.api.domain.solution.PlanningSolution; import ai.timefold.solver.core.api.score.Score; -import ai.timefold.solver.core.api.score.director.ScoreDirector; import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; -import ai.timefold.solver.core.impl.domain.variable.descriptor.BasicVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor; import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; import ai.timefold.solver.core.impl.score.definition.ScoreDefinition; @@ -109,25 +109,8 @@ public void assertScoreFromScratch(Solution_ solution) { } } - public void validateEntity(ScoreDirector scoreDirector, Object entity) { - if (listVariableDescriptor == null) { // Only basic variables. - var entityDescriptor = solutionDescriptor.findEntityDescriptorOrFail(entity.getClass()); - if (entityDescriptor.isMovable(scoreDirector.getWorkingSolution(), entity)) { - return; - } - for (var variableDescriptor : entityDescriptor.getGenuineVariableDescriptorList()) { - var basicVariableDescriptor = (BasicVariableDescriptor) variableDescriptor; - if (basicVariableDescriptor.allowsUnassigned()) { - continue; - } - var value = basicVariableDescriptor.getValue(entity); - if (value == null) { - throw new IllegalStateException( - "The entity (%s) has a variable (%s) pinned to null, even though unassigned values are not allowed." - .formatted(entity, basicVariableDescriptor.getVariableName())); - } - } - } + public Consumer getEntityValidator(InnerScoreDirector scoreDirector) { + return new EntityValidator<>(scoreDirector); } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/EntityValidator.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/EntityValidator.java new file mode 100644 index 0000000000..f99e8e3228 --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/EntityValidator.java @@ -0,0 +1,183 @@ +package ai.timefold.solver.core.impl.score.director; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; +import java.util.stream.Collectors; + +import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; +import ai.timefold.solver.core.api.score.Score; +import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; +import ai.timefold.solver.core.impl.domain.variable.descriptor.BasicVariableDescriptor; +import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor; +import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; +import ai.timefold.solver.core.impl.domain.variable.inverserelation.InverseRelationShadowVariableDescriptor; +import ai.timefold.solver.core.impl.util.CollectionUtils; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; + +final class EntityValidator> implements Consumer { + final InnerScoreDirector scoreDirector; + final SolutionDescriptor solutionDescriptor; + final ListVariableDescriptor listVariableDescriptor; + final Map, InverseRelationShadowVariableDescriptor[]> entityClassToInverseDescriptors; + final Map> valueToInverse; + + public EntityValidator(InnerScoreDirector scoreDirector) { + this.scoreDirector = scoreDirector; + this.solutionDescriptor = scoreDirector.getSolutionDescriptor(); + this.listVariableDescriptor = solutionDescriptor.getListVariableDescriptor(); + this.entityClassToInverseDescriptors = + CollectionUtils.newLinkedHashMap(solutionDescriptor.getEntityClassSet().size()); + this.valueToInverse = CollectionUtils.newIdentityHashMap(solutionDescriptor.getGenuineEntityCount( + scoreDirector.getWorkingSolution())); + + if (scoreDirector.expectShadowVariablesInCorrectState()) { + Map, VariableDescriptor[]> entityToInverseSourceDescriptors = + CollectionUtils.newLinkedHashMap(solutionDescriptor.getEntityClassSet().size()); + final int DEFAULT_INVERSE_SET_SIZE = 16; + solutionDescriptor.visitAllEntities(scoreDirector.getWorkingSolution(), + entity -> { + var entityDescriptor = solutionDescriptor.findEntityDescriptorOrFail(entity.getClass()); + @SuppressWarnings("unchecked") + var inverseSourceDescriptors = entityToInverseSourceDescriptors + .computeIfAbsent(entityDescriptor.getEntityClass(), ignored -> { + var out = new ArrayList>(); + for (var shadow : solutionDescriptor.getAllShadowVariableDescriptors()) { + if (shadow instanceof InverseRelationShadowVariableDescriptor inverseDescriptor) { + var sourceDescriptor = + inverseDescriptor.getSourceVariableDescriptorList().get(0); + if (!sourceDescriptor.isListVariable() + && sourceDescriptor.getEntityDescriptor() == entityDescriptor) { + out.add(sourceDescriptor); + } + } + } + return out.toArray(VariableDescriptor[]::new); + }); + for (var sourceDescriptor : inverseSourceDescriptors) { + var value = sourceDescriptor.getValue(entity); + valueToInverse + .computeIfAbsent(value, + ignored -> CollectionUtils.newIdentityHashSet(DEFAULT_INVERSE_SET_SIZE)) + .add(entity); + } + }); + } + } + + @Override + public void accept(Object entity) { + var entityDescriptor = solutionDescriptor.findEntityDescriptorOrFail(entity.getClass()); + + if (scoreDirector.expectShadowVariablesInCorrectState()) { + var inconsistentDescriptor = entityDescriptor.getShadowVariablesInconsistentDescriptor(); + if (inconsistentDescriptor != null) { + if (scoreDirector.expectShadowVariablesInCorrectState() + && inconsistentDescriptor.getValue(entity) == null) { + throw new IllegalStateException( + """ + Shadow variables update is disabled, but the entity (%s) has a null @%s annotated field (%s). + Maybe enable shadow variable updates? + """ + .formatted(entity, ShadowVariablesInconsistent.class.getSimpleName(), + inconsistentDescriptor.getVariableName())); + } + } + + @SuppressWarnings("unchecked") + var inverseRelationDescriptors = + entityClassToInverseDescriptors.computeIfAbsent(entityDescriptor.getEntityClass(), + ignored -> entityDescriptor.getShadowVariableDescriptors() + .stream() + .filter(descriptor -> descriptor instanceof InverseRelationShadowVariableDescriptor) + .toArray(InverseRelationShadowVariableDescriptor[]::new)); + for (var inverseRelationDescriptor : inverseRelationDescriptors) { + assertInverseRelationConsistent(scoreDirector, inverseRelationDescriptor, entity); + } + } + + if (listVariableDescriptor == null) { // Only basic variables. + if (entityDescriptor.isMovable(scoreDirector.getWorkingSolution(), entity)) { + return; + } + for (var variableDescriptor : entityDescriptor.getGenuineVariableDescriptorList()) { + var basicVariableDescriptor = (BasicVariableDescriptor) variableDescriptor; + if (basicVariableDescriptor.allowsUnassigned()) { + continue; + } + var value = basicVariableDescriptor.getValue(entity); + if (value == null) { + throw new IllegalStateException( + "The entity (%s) has a variable (%s) pinned to null, even though unassigned values are not allowed." + .formatted(entity, basicVariableDescriptor.getVariableName())); + } + } + } + } + + void assertInverseRelationConsistent(InnerScoreDirector scoreDirector, + InverseRelationShadowVariableDescriptor inverseRelationShadowVariableDescriptor, + Object entity) { + var sourceDescriptor = inverseRelationShadowVariableDescriptor.getSourceVariableDescriptorList().get(0); + if (sourceDescriptor == listVariableDescriptor) { + // skip, since ExternalizedListInverseVariableProcessor asserts this in setInverseAsserted + return; + } + + if (inverseRelationShadowVariableDescriptor.isSingleton()) { + var inverse = inverseRelationShadowVariableDescriptor.getValue(entity); + var actualSet = valueToInverse.getOrDefault(entity, Set.of()); + if (!actualSet.isEmpty() && actualSet.size() != 1) { + throw new IllegalStateException( + """ + Impossible state: The entity (%s) has a singleton @%s has multiple source entities (%s). + Verify the consistency of your input solution. + """ + .formatted(entity, InverseRelationShadowVariable.class.getSimpleName(), actualSet)); + } + if (actualSet.isEmpty() && inverse != null) { + throw new IllegalStateException( + """ + The entity (%s) has a @%s that points to a source entity (%s) but it has no source entities. + Verify the consistency of your input solution. + """ + .formatted(entity, InverseRelationShadowVariable.class.getSimpleName(), inverse)); + } + if (!actualSet.isEmpty()) { + var actual = actualSet.iterator().next(); + if (actual != inverse) { + throw new IllegalStateException( + """ + The entity (%s) has a singleton @%s that points to a source entity (%s) whereas its actual source entity is (%s). + Verify the consistency of your input solution. + """ + .formatted(entity, InverseRelationShadowVariable.class.getSimpleName(), inverse, + actual)); + } + } + } else { + @SuppressWarnings("unchecked") + var inverseCollection = (Collection) inverseRelationShadowVariableDescriptor.getValue(entity); + if (inverseCollection == null) { + throw new IllegalStateException(""" + The entity (%s) has a collection @%s that is null. + Verify the consistency of your input solution. + """.formatted(entity, InverseRelationShadowVariable.class.getSimpleName())); + } + var actualCollection = valueToInverse.getOrDefault(entity, Collections.emptySet()); + if (!inverseCollection.containsAll(actualCollection) || !actualCollection.containsAll(inverseCollection)) { + throw new IllegalStateException( + """ + The entity (%s) has a collection @%s (%s) that does not match the actual inverse set (%s). + Verify the consistency of your input solution. + """ + .formatted(entity, InverseRelationShadowVariable.class.getSimpleName(), inverseCollection, + actualCollection.stream().map(Object::toString).sorted() + .collect(Collectors.joining(", ", "[", "]")))); + } + } + } +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java index aed250cf49..963db58c54 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java @@ -83,6 +83,14 @@ static > ConstraintAnalysis getConstraintAn .toList(); } + /** + * The {@link PlanningSolution working solution} must never be the same instance as the + * {@link PlanningSolution initial solution}, it should be a clone with all shadow variables updated. + * + * @param initialSolution never null + */ + void setInitialSolution(Solution_ initialSolution); + /** * The {@link PlanningSolution working solution} must never be the same instance as the * {@link PlanningSolution best solution}, it should be a (un)changed clone. @@ -209,16 +217,6 @@ static > ConstraintAnalysis getConstraintAn */ boolean expectShadowVariablesInCorrectState(); - /** - * Disables the checks controlled by {@link #expectShadowVariablesInCorrectState()}. - */ - void disableShadowVariablesInCorrectStateChecks(); - - /** - * Enables the checks controlled by {@link #expectShadowVariablesInCorrectState()}. - */ - void enableShadowVariablesInCorrectStateChecks(); - /** * @return never null */ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolver.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolver.java index 87d917905d..91aca1bab8 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolver.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolver.java @@ -180,7 +180,7 @@ public void setMonitorTagMap(Map monitorTagMap) { var solveLengthTimer = Metrics.more().longTaskTimer(SolverMetric.SOLVE_DURATION.getMeterId()); var errorCounter = Metrics.counter(SolverMetric.ERROR_COUNT.getMeterId()); - solverScope.setBestSolution(Objects.requireNonNull(problem, "The problem must not be null.")); + solverScope.setInitialSolution(Objects.requireNonNull(problem, "The problem must not be null.")); solverScope.setSolver(this); outerSolvingStarted(solverScope); var restartSolver = true; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java index 541afdf86c..2e7f975c30 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java @@ -332,12 +332,15 @@ public long getMoveEvaluationSpeed() { public void setWorkingSolutionFromBestSolution() { // The workingSolution must never be the same instance as the bestSolution. - scoreDirector.disableShadowVariablesInCorrectStateChecks(); scoreDirector.setWorkingSolution(scoreDirector.cloneSolution(getBestSolution())); - // The best solution might have some stale shadow variables, so - // update all shadow variables - scoreDirector.forceTriggerVariableListeners(); - scoreDirector.enableShadowVariablesInCorrectStateChecks(); + } + + public void setInitialSolution(Solution_ initialSolution) { + // The workingSolution must never be the same instance as the bestSolution. + scoreDirector.setInitialSolution(scoreDirector.cloneSolution(initialSolution)); + + // Set the best solution to the solution with shadow variable updated! + setBestSolution(scoreDirector.cloneSolution(scoreDirector.getWorkingSolution())); } public SolverScope createChildThreadSolverScope(ChildThreadType childThreadType) { diff --git a/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java b/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java index f3260757f6..f2499fb808 100644 --- a/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java +++ b/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java @@ -477,7 +477,7 @@ void updateOnlyScoreFailsIfInverseCollectionMissingElements(SolutionManagerSourc "The entity (v1)", "has a collection @" + InverseRelationShadowVariable.class.getSimpleName(), "([e1])", - "but it is missing a source entity that point to it (e2)"); + "that does not match the actual inverse set ([e1, e2])"); } @ParameterizedTest @@ -515,8 +515,7 @@ void updateOnlyScoreFailsIfInverseCollectionHasExtraElements(SolutionManagerSour "The entity (v2)", "has a collection @" + InverseRelationShadowVariable.class.getSimpleName(), "([e3, e1])", - "that points to a source entity (e1)", - "but that source entity points to (v1) instead"); + "that does not match the actual inverse set ([e3])"); } @ParameterizedTest diff --git a/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java index c96c176330..ad3645c9cf 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java @@ -104,7 +104,9 @@ void solveWithInitializedSolution() { new TestdataEntity("e3", v3))); var solution = PlannerTestUtils.solve(solverConfig, inputProblem, false); - assertThat(inputProblem).isSameAs(solution); + // Although the solution has not changed, it is a clone since the initial solution + // may have stale shadow variables. + assertThat(inputProblem).isNotSameAs(solution); } @Test From 94de76f3592c2fa9d4cfc8ccff91f1a9d82d65aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Wed, 20 Aug 2025 08:47:49 +0200 Subject: [PATCH 3/6] chore: add Javadoc on update policy implications --- .../solver/core/api/score/ScoreManager.java | 14 +++++++++++++- .../core/api/solver/SolutionUpdatePolicy.java | 10 ++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/api/score/ScoreManager.java b/core/src/main/java/ai/timefold/solver/core/api/score/ScoreManager.java index 9a2e0dbafa..efd37cab60 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/score/ScoreManager.java +++ b/core/src/main/java/ai/timefold/solver/core/api/score/ScoreManager.java @@ -67,6 +67,12 @@ static , ProblemId_> ScoreManager + * It is equivalent to calling {@link #update(Object, SolutionUpdatePolicy)} + * with {@link SolutionUpdatePolicy#UPDATE_SCORE_ONLY}. + * This policy doesn't update shadow variables, which carries a performance advantage + * but also brings additional limitations. + * Please review the {@link SolutionUpdatePolicy} documentation for details. * * @param solution never null */ @@ -77,7 +83,7 @@ static , ProblemId_> ScoreManager - * Do not parse this string. + * Don't parse this string. * Instead, to provide this information in a UI or a service, use {@link #explainScore(Object)} * to retrieve {@link ScoreExplanation#getConstraintMatchTotalMap()} and {@link ScoreExplanation#getIndictmentMap()} * and convert those into a domain specific API. @@ -92,6 +98,12 @@ static , ProblemId_> ScoreManager + * It is equivalent to calling {@link #explain(Object, SolutionUpdatePolicy)} + * with {@link SolutionUpdatePolicy#UPDATE_SCORE_ONLY}. + * This policy doesn't update shadow variables, which carries a performance advantage + * but also brings additional limitations. + * Please review the {@link SolutionUpdatePolicy} documentation for details. * * @param solution never null * @return never null diff --git a/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java b/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java index 8b4d8d8d97..8c92f72303 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java +++ b/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java @@ -31,10 +31,12 @@ public enum SolutionUpdatePolicy { /** * Calculates the score based on the entities in the solution, * and writes it back to the solution. - * Does not trigger shadow variables; - * if score calculation requires shadow variable values, - * {@link NullPointerException} is likely to be thrown. - * To avoid this, use {@link #UPDATE_ALL} instead. + * Does not update shadow variables, + * making the user responsible for ensuring that all shadow variables are mutually consistent. + * Otherwise the results of the computation are undefined, + * and may range from the wrong score being computed to runtime exceptions being thrown. + * To avoid this issue, use {@link #UPDATE_ALL} instead, + * which will update shadow variables to their correct values first. */ UPDATE_SCORE_ONLY(true, false), /** From e3ca804c24208ddd61cb2b4c8f56ac4456d3962b Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Wed, 20 Aug 2025 14:53:17 -0400 Subject: [PATCH 4/6] chore: review comments --- .../declarative/AffectedEntitiesUpdater.java | 12 +- .../declarative/RootVariableSource.java | 2 +- .../score/director/AbstractScoreDirector.java | 2 +- .../AbstractScoreDirectorFactory.java | 25 ++- .../impl/score/director/EntityValidator.java | 183 ------------------ .../core/impl/solver/DefaultSolver.java | 5 + .../ShadowVariablesInconsistent.java | 4 +- .../core/api/solver/SolutionManagerTest.java | 176 ----------------- .../config/solver/EnvironmentModeTest.java | 2 +- ...DefaultConstructionHeuristicPhaseTest.java | 8 +- .../declarative/RootVariableSourceTest.java | 2 +- .../DefaultExhaustiveSearchPhaseTest.java | 4 +- .../DefaultLocalSearchPhaseTest.java | 14 +- .../core/impl/solver/DefaultSolverTest.java | 10 +- 14 files changed, 56 insertions(+), 393 deletions(-) delete mode 100644 core/src/main/java/ai/timefold/solver/core/impl/score/director/EntityValidator.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java index db9bd7f277..0f968b9523 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java @@ -120,13 +120,13 @@ private void initializeChangeQueue(BitSet changed) { changed.clear(); } - private boolean updateEntityShadowVariables(GraphNode entityVariable, boolean isVariableLooped) { + private boolean updateEntityShadowVariables(GraphNode entityVariable, boolean isVariableInconsistent) { var entity = entityVariable.entity(); var shadowVariableReferences = entityVariable.variableReferences(); - var loopDescriptor = shadowVariableReferences.get(0).shadowVariablesInconsistentDescriptor(); + var inconsistentDescriptor = shadowVariableReferences.get(0).shadowVariablesInconsistentDescriptor(); var anyChanged = false; - if (loopDescriptor != null) { + if (inconsistentDescriptor != null) { // Do not need to update anyChanged here; the graph already marked // all nodes whose looped status changed for us var groupEntities = shadowVariableReferences.get(0).groupEntities(); @@ -136,15 +136,15 @@ private boolean updateEntityShadowVariables(GraphNode entityVariable, for (var i = 0; i < groupEntityIds.length; i++) { var groupEntity = groupEntities[i]; var groupEntityId = groupEntityIds[i]; - anyChanged |= updateLoopedStatusOfEntity(groupEntity, groupEntityId, loopDescriptor); + anyChanged |= updateLoopedStatusOfEntity(groupEntity, groupEntityId, inconsistentDescriptor); } } else { - anyChanged |= updateLoopedStatusOfEntity(entity, entityVariable.entityId(), loopDescriptor); + anyChanged |= updateLoopedStatusOfEntity(entity, entityVariable.entityId(), inconsistentDescriptor); } } for (var shadowVariableReference : shadowVariableReferences) { - anyChanged |= updateShadowVariable(isVariableLooped, shadowVariableReference, entity); + anyChanged |= updateShadowVariable(isVariableInconsistent, shadowVariableReference, entity); } return anyChanged; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java index 979903174c..66e6dbc6cb 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java @@ -399,7 +399,7 @@ private static ParentVariableType determineParentVariableType(Class rootClass if (getAnnotation(declaringClass, memberName, ShadowVariablesInconsistent.class) != null) { throw new IllegalArgumentException(""" The source path (%s) starting from root class (%s) accesses a @%s property (%s). - Supplier methods are only called when none of their dependencies are inconsistent, + Supplier methods are only called when all of their dependencies are consistent, so reading @%s properties are not needed since they are guaranteed to be false when the supplier is called. Maybe remove the source path (%s) from the @%s? diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java index 86d48221a8..5f9f1d32ed 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java @@ -251,7 +251,7 @@ protected void setWorkingSolution(Solution_ workingSolution, Consumer en if (entityAndFactVisitor != null) { solutionDescriptor.visitAllProblemFacts(workingSolution, entityAndFactVisitor); } - Consumer entityValidator = scoreDirectorFactory.getEntityValidator(this); + Consumer entityValidator = entity -> scoreDirectorFactory.validateEntity(this, entity); entityAndFactVisitor = entityAndFactVisitor == null ? entityValidator : entityAndFactVisitor.andThen(entityValidator); setWorkingEntityListDirty(workingSolution); // This visits all the entities. 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 2c8e862a50..f2b9bc7d15 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 @@ -1,11 +1,11 @@ package ai.timefold.solver.core.impl.score.director; -import java.util.function.Consumer; - import ai.timefold.solver.core.api.domain.solution.PlanningSolution; import ai.timefold.solver.core.api.score.Score; +import ai.timefold.solver.core.api.score.director.ScoreDirector; import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; +import ai.timefold.solver.core.impl.domain.variable.descriptor.BasicVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor; import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; import ai.timefold.solver.core.impl.score.definition.ScoreDefinition; @@ -109,8 +109,25 @@ public void assertScoreFromScratch(Solution_ solution) { } } - public Consumer getEntityValidator(InnerScoreDirector scoreDirector) { - return new EntityValidator<>(scoreDirector); + public void validateEntity(ScoreDirector scoreDirector, Object entity) { + if (listVariableDescriptor == null) { // Only basic variables. + var entityDescriptor = solutionDescriptor.findEntityDescriptorOrFail(entity.getClass()); + if (entityDescriptor.isMovable(scoreDirector.getWorkingSolution(), entity)) { + return; + } + for (var variableDescriptor : entityDescriptor.getGenuineVariableDescriptorList()) { + var basicVariableDescriptor = (BasicVariableDescriptor) variableDescriptor; + if (basicVariableDescriptor.allowsUnassigned()) { + continue; + } + var value = basicVariableDescriptor.getValue(entity); + if (value == null) { + throw new IllegalStateException( + "The entity (%s) has a variable (%s) pinned to null, even though unassigned values are not allowed." + .formatted(entity, basicVariableDescriptor.getVariableName())); + } + } + } } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/EntityValidator.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/EntityValidator.java deleted file mode 100644 index f99e8e3228..0000000000 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/EntityValidator.java +++ /dev/null @@ -1,183 +0,0 @@ -package ai.timefold.solver.core.impl.score.director; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Map; -import java.util.Set; -import java.util.function.Consumer; -import java.util.stream.Collectors; - -import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; -import ai.timefold.solver.core.api.score.Score; -import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; -import ai.timefold.solver.core.impl.domain.variable.descriptor.BasicVariableDescriptor; -import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor; -import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; -import ai.timefold.solver.core.impl.domain.variable.inverserelation.InverseRelationShadowVariableDescriptor; -import ai.timefold.solver.core.impl.util.CollectionUtils; -import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowVariablesInconsistent; - -final class EntityValidator> implements Consumer { - final InnerScoreDirector scoreDirector; - final SolutionDescriptor solutionDescriptor; - final ListVariableDescriptor listVariableDescriptor; - final Map, InverseRelationShadowVariableDescriptor[]> entityClassToInverseDescriptors; - final Map> valueToInverse; - - public EntityValidator(InnerScoreDirector scoreDirector) { - this.scoreDirector = scoreDirector; - this.solutionDescriptor = scoreDirector.getSolutionDescriptor(); - this.listVariableDescriptor = solutionDescriptor.getListVariableDescriptor(); - this.entityClassToInverseDescriptors = - CollectionUtils.newLinkedHashMap(solutionDescriptor.getEntityClassSet().size()); - this.valueToInverse = CollectionUtils.newIdentityHashMap(solutionDescriptor.getGenuineEntityCount( - scoreDirector.getWorkingSolution())); - - if (scoreDirector.expectShadowVariablesInCorrectState()) { - Map, VariableDescriptor[]> entityToInverseSourceDescriptors = - CollectionUtils.newLinkedHashMap(solutionDescriptor.getEntityClassSet().size()); - final int DEFAULT_INVERSE_SET_SIZE = 16; - solutionDescriptor.visitAllEntities(scoreDirector.getWorkingSolution(), - entity -> { - var entityDescriptor = solutionDescriptor.findEntityDescriptorOrFail(entity.getClass()); - @SuppressWarnings("unchecked") - var inverseSourceDescriptors = entityToInverseSourceDescriptors - .computeIfAbsent(entityDescriptor.getEntityClass(), ignored -> { - var out = new ArrayList>(); - for (var shadow : solutionDescriptor.getAllShadowVariableDescriptors()) { - if (shadow instanceof InverseRelationShadowVariableDescriptor inverseDescriptor) { - var sourceDescriptor = - inverseDescriptor.getSourceVariableDescriptorList().get(0); - if (!sourceDescriptor.isListVariable() - && sourceDescriptor.getEntityDescriptor() == entityDescriptor) { - out.add(sourceDescriptor); - } - } - } - return out.toArray(VariableDescriptor[]::new); - }); - for (var sourceDescriptor : inverseSourceDescriptors) { - var value = sourceDescriptor.getValue(entity); - valueToInverse - .computeIfAbsent(value, - ignored -> CollectionUtils.newIdentityHashSet(DEFAULT_INVERSE_SET_SIZE)) - .add(entity); - } - }); - } - } - - @Override - public void accept(Object entity) { - var entityDescriptor = solutionDescriptor.findEntityDescriptorOrFail(entity.getClass()); - - if (scoreDirector.expectShadowVariablesInCorrectState()) { - var inconsistentDescriptor = entityDescriptor.getShadowVariablesInconsistentDescriptor(); - if (inconsistentDescriptor != null) { - if (scoreDirector.expectShadowVariablesInCorrectState() - && inconsistentDescriptor.getValue(entity) == null) { - throw new IllegalStateException( - """ - Shadow variables update is disabled, but the entity (%s) has a null @%s annotated field (%s). - Maybe enable shadow variable updates? - """ - .formatted(entity, ShadowVariablesInconsistent.class.getSimpleName(), - inconsistentDescriptor.getVariableName())); - } - } - - @SuppressWarnings("unchecked") - var inverseRelationDescriptors = - entityClassToInverseDescriptors.computeIfAbsent(entityDescriptor.getEntityClass(), - ignored -> entityDescriptor.getShadowVariableDescriptors() - .stream() - .filter(descriptor -> descriptor instanceof InverseRelationShadowVariableDescriptor) - .toArray(InverseRelationShadowVariableDescriptor[]::new)); - for (var inverseRelationDescriptor : inverseRelationDescriptors) { - assertInverseRelationConsistent(scoreDirector, inverseRelationDescriptor, entity); - } - } - - if (listVariableDescriptor == null) { // Only basic variables. - if (entityDescriptor.isMovable(scoreDirector.getWorkingSolution(), entity)) { - return; - } - for (var variableDescriptor : entityDescriptor.getGenuineVariableDescriptorList()) { - var basicVariableDescriptor = (BasicVariableDescriptor) variableDescriptor; - if (basicVariableDescriptor.allowsUnassigned()) { - continue; - } - var value = basicVariableDescriptor.getValue(entity); - if (value == null) { - throw new IllegalStateException( - "The entity (%s) has a variable (%s) pinned to null, even though unassigned values are not allowed." - .formatted(entity, basicVariableDescriptor.getVariableName())); - } - } - } - } - - void assertInverseRelationConsistent(InnerScoreDirector scoreDirector, - InverseRelationShadowVariableDescriptor inverseRelationShadowVariableDescriptor, - Object entity) { - var sourceDescriptor = inverseRelationShadowVariableDescriptor.getSourceVariableDescriptorList().get(0); - if (sourceDescriptor == listVariableDescriptor) { - // skip, since ExternalizedListInverseVariableProcessor asserts this in setInverseAsserted - return; - } - - if (inverseRelationShadowVariableDescriptor.isSingleton()) { - var inverse = inverseRelationShadowVariableDescriptor.getValue(entity); - var actualSet = valueToInverse.getOrDefault(entity, Set.of()); - if (!actualSet.isEmpty() && actualSet.size() != 1) { - throw new IllegalStateException( - """ - Impossible state: The entity (%s) has a singleton @%s has multiple source entities (%s). - Verify the consistency of your input solution. - """ - .formatted(entity, InverseRelationShadowVariable.class.getSimpleName(), actualSet)); - } - if (actualSet.isEmpty() && inverse != null) { - throw new IllegalStateException( - """ - The entity (%s) has a @%s that points to a source entity (%s) but it has no source entities. - Verify the consistency of your input solution. - """ - .formatted(entity, InverseRelationShadowVariable.class.getSimpleName(), inverse)); - } - if (!actualSet.isEmpty()) { - var actual = actualSet.iterator().next(); - if (actual != inverse) { - throw new IllegalStateException( - """ - The entity (%s) has a singleton @%s that points to a source entity (%s) whereas its actual source entity is (%s). - Verify the consistency of your input solution. - """ - .formatted(entity, InverseRelationShadowVariable.class.getSimpleName(), inverse, - actual)); - } - } - } else { - @SuppressWarnings("unchecked") - var inverseCollection = (Collection) inverseRelationShadowVariableDescriptor.getValue(entity); - if (inverseCollection == null) { - throw new IllegalStateException(""" - The entity (%s) has a collection @%s that is null. - Verify the consistency of your input solution. - """.formatted(entity, InverseRelationShadowVariable.class.getSimpleName())); - } - var actualCollection = valueToInverse.getOrDefault(entity, Collections.emptySet()); - if (!inverseCollection.containsAll(actualCollection) || !actualCollection.containsAll(inverseCollection)) { - throw new IllegalStateException( - """ - The entity (%s) has a collection @%s (%s) that does not match the actual inverse set (%s). - Verify the consistency of your input solution. - """ - .formatted(entity, InverseRelationShadowVariable.class.getSimpleName(), inverseCollection, - actualCollection.stream().map(Object::toString).sorted() - .collect(Collectors.joining(", ", "[", "]")))); - } - } - } -} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolver.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolver.java index 91aca1bab8..3e33138609 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolver.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolver.java @@ -183,6 +183,7 @@ public void setMonitorTagMap(Map monitorTagMap) { solverScope.setInitialSolution(Objects.requireNonNull(problem, "The problem must not be null.")); solverScope.setSolver(this); outerSolvingStarted(solverScope); + var restartSolver = true; while (restartSolver) { var sample = solveLengthTimer.start(); @@ -223,6 +224,10 @@ public void solvingStarted(SolverScope solverScope) { var startingSolverCount = solverScope.getStartingSolverCount() + 1; solverScope.setStartingSolverCount(startingSolverCount); registerSolverSpecificMetrics(); + + // Update the best solution, since problem's shadows and score were updated + bestSolutionRecaller.updateBestSolutionAndFireIfInitialized(solverScope); + logger.info("Solving {}: time spent ({}), best score ({}), " + "environment mode ({}), move thread count ({}), random ({}).", (startingSolverCount == 1 ? "started" : "restarted"), diff --git a/core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/ShadowVariablesInconsistent.java b/core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/ShadowVariablesInconsistent.java index a6a2d4f915..f1ff358905 100644 --- a/core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/ShadowVariablesInconsistent.java +++ b/core/src/main/java/ai/timefold/solver/core/preview/api/domain/variable/declarative/ShadowVariablesInconsistent.java @@ -24,7 +24,7 @@ * `a` depends on `b` and `b` depends on `a`). * *
  • - * One of its source variables is looped (for example, + * One of its source variables is inconsistent (for example, * `c` depends on `a`, which depends on `b`, and `b` depends on `a`). *
    • * @@ -39,7 +39,7 @@ * be updated after the {@link ShadowSources} marked method is called, causing * score corruption. {@link ShadowSources} marked methods do not need to check * {@link ShadowVariablesInconsistent} properties, since they are only called if all - * their dependencies are not looped. + * their dependencies are consistent. */ @Target({ METHOD, FIELD }) @Retention(RUNTIME) diff --git a/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java b/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java index f2499fb808..b70ec3c1e8 100644 --- a/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java +++ b/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java @@ -9,7 +9,6 @@ import java.util.List; import java.util.function.Function; -import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; import ai.timefold.solver.core.api.score.Score; import ai.timefold.solver.core.api.score.buildin.hardsoft.HardSoftScore; import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore; @@ -343,181 +342,6 @@ void updateOnlyScoreDeclarativeShadows(SolutionManagerSource SolutionManagerSour assertThat(solution.getScore()).isEqualTo(HardSoftScore.ofHard(-4)); } - @ParameterizedTest - @EnumSource(SolutionManagerSource.class) - void updateOnlyScoreFailsIfShadowVariablesInconsistentIsNull(SolutionManagerSource SolutionManagerSource) { - var solution = new TestdataConcurrentSolution(); - var e1 = new TestdataConcurrentEntity("e1"); - var e2 = new TestdataConcurrentEntity("e2"); - - var a1 = new TestdataConcurrentValue("a1"); - var a2 = new TestdataConcurrentValue("a2"); - var b1 = new TestdataConcurrentValue("b1"); - var b2 = new TestdataConcurrentValue("b2"); - - var groupA = List.of(a1, a2); - var groupB = List.of(b1, b2); - - var entities = List.of(e1, e2); - var values = List.of(a1, a2, b1, b2); - - a1.setConcurrentValueGroup(groupA); - a2.setConcurrentValueGroup(groupA); - - b1.setConcurrentValueGroup(groupB); - b2.setConcurrentValueGroup(groupB); - - e1.setValues(List.of(a1, b1)); - e2.setValues(List.of(b2, a2)); - - b1.setPreviousValue(a1); - a2.setPreviousValue(b2); - - a1.setNextValue(b1); - b2.setNextValue(a2); - - a1.setIndex(0); - a2.setIndex(1); - - b1.setIndex(1); - b2.setIndex(0); - - a1.setEntity(e1); - b1.setEntity(e1); - a2.setEntity(e2); - b2.setEntity(e2); - - a1.setInconsistent(true); - a2.setInconsistent(null); - b1.setInconsistent(true); - b2.setInconsistent(true); - - solution.setEntities(entities); - solution.setValues(values); - - var solutionManager = SolutionManagerSource.createSolutionManager(SOLVER_FACTORY_DECLARATIVE_SHADOW); - assertThat(solutionManager).isNotNull(); - assertThatCode(() -> { - solutionManager.update(solution, SolutionUpdatePolicy.UPDATE_SCORE_ONLY); - }).hasMessageContainingAll( - "Shadow variables update is disabled", - "but the entity (e2 -> b2 -> a2)", - "has a null @ShadowVariablesInconsistent annotated field (isInconsistent)"); - } - - @ParameterizedTest - @EnumSource(SolutionManagerSource.class) - void updateOnlyScoreFailsIfInverseCollectionNull(SolutionManagerSource SolutionManagerSource) { - var solution = new TestdataInverseRelationSolution("solution"); - var e1 = new TestdataInverseRelationEntity("e1"); - var e2 = new TestdataInverseRelationEntity("e2"); - var e3 = new TestdataInverseRelationEntity("e3"); - - var v1 = new TestdataInverseRelationValue("v1"); - var v2 = new TestdataInverseRelationValue("v2"); - - v1.setEntities(null); - v2.setEntities(null); - - solution.setEntityList(List.of(e1, e2, e3)); - solution.setValueList(List.of(v1, v2)); - - e1.setValue(v1); - e2.setValue(v1); - e3.setValue(v2); - - assertSoftly(softly -> { - softly.assertThat(solution.getScore()).isNull(); - softly.assertThat(solution.getValueList().get(0).getEntities()).isNull(); - softly.assertThat(solution.getValueList().get(1).getEntities()).isNull(); - }); - - var solutionManager = SolutionManagerSource.createSolutionManager(SOLVER_FACTORY_INVERSE_RELATION); - assertThat(solutionManager).isNotNull(); - assertThatCode(() -> { - solutionManager.update(solution, SolutionUpdatePolicy.UPDATE_SCORE_ONLY); - }).hasMessageContainingAll( - "The entity (v1)", - "has a collection @" + InverseRelationShadowVariable.class.getSimpleName(), - "that is null"); - } - - @ParameterizedTest - @EnumSource(SolutionManagerSource.class) - void updateOnlyScoreFailsIfInverseCollectionMissingElements(SolutionManagerSource SolutionManagerSource) { - var solution = new TestdataInverseRelationSolution("solution"); - var e1 = new TestdataInverseRelationEntity("e1"); - var e2 = new TestdataInverseRelationEntity("e2"); - var e3 = new TestdataInverseRelationEntity("e3"); - - var v1 = new TestdataInverseRelationValue("v1"); - var v2 = new TestdataInverseRelationValue("v2"); - - v1.setEntities(List.of(e1)); - v2.setEntities(List.of(e3)); - - solution.setEntityList(List.of(e1, e2, e3)); - solution.setValueList(List.of(v1, v2)); - - e1.setValue(v1); - e2.setValue(v1); - e3.setValue(v2); - - assertSoftly(softly -> { - softly.assertThat(solution.getScore()).isNull(); - softly.assertThat(solution.getValueList().get(0).getEntities()).containsExactly(e1); - softly.assertThat(solution.getValueList().get(1).getEntities()).containsExactly(e3); - }); - - var solutionManager = SolutionManagerSource.createSolutionManager(SOLVER_FACTORY_INVERSE_RELATION); - assertThat(solutionManager).isNotNull(); - assertThatCode(() -> { - solutionManager.update(solution, SolutionUpdatePolicy.UPDATE_SCORE_ONLY); - }).hasMessageContainingAll( - "The entity (v1)", - "has a collection @" + InverseRelationShadowVariable.class.getSimpleName(), - "([e1])", - "that does not match the actual inverse set ([e1, e2])"); - } - - @ParameterizedTest - @EnumSource(SolutionManagerSource.class) - void updateOnlyScoreFailsIfInverseCollectionHasExtraElements(SolutionManagerSource SolutionManagerSource) { - var solution = new TestdataInverseRelationSolution("solution"); - var e1 = new TestdataInverseRelationEntity("e1"); - var e2 = new TestdataInverseRelationEntity("e2"); - var e3 = new TestdataInverseRelationEntity("e3"); - - var v1 = new TestdataInverseRelationValue("v1"); - var v2 = new TestdataInverseRelationValue("v2"); - - v1.setEntities(List.of(e1, e2)); - v2.setEntities(List.of(e3, e1)); - - solution.setEntityList(List.of(e1, e2, e3)); - solution.setValueList(List.of(v1, v2)); - - e1.setValue(v1); - e2.setValue(v1); - e3.setValue(v2); - - assertSoftly(softly -> { - softly.assertThat(solution.getScore()).isNull(); - softly.assertThat(solution.getValueList().get(0).getEntities()).containsExactly(e1, e2); - softly.assertThat(solution.getValueList().get(1).getEntities()).containsExactly(e3, e1); - }); - - var solutionManager = SolutionManagerSource.createSolutionManager(SOLVER_FACTORY_INVERSE_RELATION); - assertThat(solutionManager).isNotNull(); - assertThatCode(() -> { - solutionManager.update(solution, SolutionUpdatePolicy.UPDATE_SCORE_ONLY); - }).hasMessageContainingAll( - "The entity (v2)", - "has a collection @" + InverseRelationShadowVariable.class.getSimpleName(), - "([e3, e1])", - "that does not match the actual inverse set ([e3])"); - } - @ParameterizedTest @EnumSource(SolutionManagerSource.class) void updateOnlyScoreFailsIfListVariableInconsistent(SolutionManagerSource SolutionManagerSource) { diff --git a/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java b/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java index 19922f2b9b..a6395b3ae9 100644 --- a/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java +++ b/core/src/test/java/ai/timefold/solver/core/config/solver/EnvironmentModeTest.java @@ -159,7 +159,7 @@ void corruptedUndoShadowVariableListener(EnvironmentMode environmentMode) { v2.setEntities(new ArrayList<>(List.of(e2))); assertThatNoException() .isThrownBy(() -> PlannerTestUtils.solve(solverConfig, - new CorruptedUndoShadowSolution(List.of(e1, e2), List.of(v1, v2)), false)); + new CorruptedUndoShadowSolution(List.of(e1, e2), List.of(v1, v2)), true)); } } } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java index ad3645c9cf..1d7aad09d5 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java @@ -103,7 +103,7 @@ void solveWithInitializedSolution() { new TestdataEntity("e2", v2), new TestdataEntity("e3", v3))); - var solution = PlannerTestUtils.solve(solverConfig, inputProblem, false); + var solution = PlannerTestUtils.solve(solverConfig, inputProblem, true); // Although the solution has not changed, it is a clone since the initial solution // may have stale shadow variables. assertThat(inputProblem).isNotSameAs(solution); @@ -230,7 +230,7 @@ void solveWithPinnedEntitiesWhenUnassignedAllowedAndPinnedToNull() { new TestdataPinnedAllowsUnassignedEntity("e2", v2, true, false), new TestdataPinnedAllowsUnassignedEntity("e3", null, false, true))); - solution = PlannerTestUtils.solve(solverConfig, solution, false); // No change will be made. + solution = PlannerTestUtils.solve(solverConfig, solution, true); // No change will be made, but shadow variables will be updated. assertThat(solution).isNotNull(); assertThat(solution.getScore()).isEqualTo(SimpleScore.ZERO); } @@ -268,7 +268,7 @@ void solveWithEmptyEntityList() { solution.setValueList(Arrays.asList(v1, v2, v3)); solution.setEntityList(Collections.emptyList()); - solution = PlannerTestUtils.solve(solverConfig, solution, false); + solution = PlannerTestUtils.solve(solverConfig, solution, true); assertThat(solution).isNotNull(); assertThat(solution.getEntityList()).isEmpty(); } @@ -327,7 +327,7 @@ void solveWithAllowsUnassignedValuesListVariable() { solution.setEntityList(List.of(entity)); solution.setValueList(Arrays.asList(value1, value2, value3, value4)); - var bestSolution = PlannerTestUtils.solve(solverConfig, solution, false); + var bestSolution = PlannerTestUtils.solve(solverConfig, solution, true); assertSoftly(softly -> { softly.assertThat(bestSolution.getScore()) .isEqualTo(SimpleScore.of(-2)); // Length of the entity's value list. diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java index 8aaa466ed1..d7e23b1861 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java @@ -709,7 +709,7 @@ void errorIfShadowVariablesInconsistentReferenced() { "The source path (isInconsistent) starting from root class (%s) accesses a @%s property (isInconsistent)" .formatted(TestdataInvalidDeclarativeValue.class.getCanonicalName(), ShadowVariablesInconsistent.class.getSimpleName()), - "Supplier methods are only called when none of their dependencies are inconsistent", + "Supplier methods are only called when all of their dependencies are consistent", "reading @%s properties are not needed since they are guaranteed to be false" .formatted(ShadowVariablesInconsistent.class.getSimpleName()), "Maybe remove the source path (isInconsistent) from the @%s?" diff --git a/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java index 4b192ae0f3..e2f08fd3ad 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhaseTest.java @@ -277,7 +277,7 @@ void solveWithPinnedEntitiesWhenUnassignedAllowedAndPinnedToNull() { new TestdataPinnedAllowsUnassignedEntity("e2", v2, true, false), new TestdataPinnedAllowsUnassignedEntity("e3", null, false, true))); - solution = PlannerTestUtils.solve(solverConfig, solution, false); // No change will be made. + solution = PlannerTestUtils.solve(solverConfig, solution, true); // No change will be made, but shadows will be updated. assertThat(solution).isNotNull(); assertThat(solution.getScore()).isEqualTo(SimpleScore.ZERO); } @@ -315,7 +315,7 @@ void solveWithEmptyEntityList() { solution.setValueList(Arrays.asList(v1, v2, v3)); solution.setEntityList(Collections.emptyList()); - solution = PlannerTestUtils.solve(solverConfig, solution, false); + solution = PlannerTestUtils.solve(solverConfig, solution, true); assertThat(solution).isNotNull(); assertThat(solution.getEntityList()).isEmpty(); } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java b/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java index 6f3d8c2ff8..e41b6f39e9 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java @@ -58,7 +58,7 @@ void solveWithInitializedEntities() { new TestdataEntity("e2", v2), new TestdataEntity("e3", v1))); - solution = PlannerTestUtils.solve(solverConfig, solution, false); // TODO incentive it to change something + solution = PlannerTestUtils.solve(solverConfig, solution, true); // TODO incentive it to change something assertThat(solution).isNotNull(); var solvedE1 = solution.getEntityList().get(0); assertCode("e1", solvedE1); @@ -190,7 +190,7 @@ void solveWithPinnedEntities() { new TestdataPinnedEntity("e2", v2, true, false), new TestdataPinnedEntity("e3", v3, false, true))); - solution = PlannerTestUtils.solve(solverConfig, solution, false); // TODO incentive it to change something + solution = PlannerTestUtils.solve(solverConfig, solution, true); // TODO incentive it to change something assertThat(solution).isNotNull(); var solvedE1 = solution.getEntityList().get(0); assertCode("e1", solvedE1); @@ -220,7 +220,7 @@ void solveWithPinnedEntitiesWhenUnassignedAllowedAndPinnedToNull() { new TestdataPinnedAllowsUnassignedEntity("e2", v2, true, false), new TestdataPinnedAllowsUnassignedEntity("e3", null, false, true))); - solution = PlannerTestUtils.solve(solverConfig, solution, false); // No change will be made. + solution = PlannerTestUtils.solve(solverConfig, solution, true); // No change will be made. assertThat(solution).isNotNull(); assertThat(solution.getScore()).isEqualTo(SimpleScore.ZERO); } @@ -260,7 +260,7 @@ void solveWithEmptyEntityList() { solution.setValueList(Arrays.asList(v1, v2, v3)); solution.setEntityList(Collections.emptyList()); - solution = PlannerTestUtils.solve(solverConfig, solution, false); + solution = PlannerTestUtils.solve(solverConfig, solution, true); assertThat(solution).isNotNull(); assertThat(solution.getEntityList()).isEmpty(); } @@ -283,7 +283,7 @@ void solveTabuSearchWithInitializedEntities() { new TestdataEntity("e2", v2), new TestdataEntity("e3", v1))); - solution = PlannerTestUtils.solve(solverConfig, solution, false); // TODO incentive it to change something + solution = PlannerTestUtils.solve(solverConfig, solution, true); // TODO incentive it to change something assertThat(solution).isNotNull(); var solvedE1 = solution.getEntityList().get(0); assertCode("e1", solvedE1); @@ -314,7 +314,7 @@ void solveTabuSearchWithPinnedEntities() { new TestdataPinnedEntity("e2", v2, true, false), new TestdataPinnedEntity("e3", v3, false, true))); - solution = PlannerTestUtils.solve(solverConfig, solution, false); // TODO incentive it to change something + solution = PlannerTestUtils.solve(solverConfig, solution, true); // TODO incentive it to change something assertThat(solution).isNotNull(); var solvedE1 = solution.getEntityList().get(0); assertCode("e1", solvedE1); @@ -342,7 +342,7 @@ void solveTabuSearchWithEmptyEntityList() { solution.setValueList(Arrays.asList(v1, v2, v3)); solution.setEntityList(Collections.emptyList()); - solution = PlannerTestUtils.solve(solverConfig, solution, false); + solution = PlannerTestUtils.solve(solverConfig, solution, true); assertThat(solution).isNotNull(); assertThat(solution.getEntityList()).isEmpty(); } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java index 1dfb329987..16c54a7560 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java @@ -954,7 +954,7 @@ void solveEmptyEntityList() { solution.setValueList(Arrays.asList(new TestdataValue("v1"), new TestdataValue("v2"))); solution.setEntityList(Collections.emptyList()); - solution = PlannerTestUtils.solve(solverConfig, solution, false); + solution = PlannerTestUtils.solve(solverConfig, solution, true); assertThat(solution).isNotNull(); assertThat(solution.getEntityList().stream() .filter(e -> e.getValue() == null)).isEmpty(); @@ -979,7 +979,7 @@ void solveChainedEmptyEntityList() { solution.setChainedAnchorList(Arrays.asList(new TestdataChainedAnchor("v1"), new TestdataChainedAnchor("v2"))); solution.setChainedEntityList(Collections.emptyList()); - solution = PlannerTestUtils.solve(solverConfig, solution, false); + solution = PlannerTestUtils.solve(solverConfig, solution, true); assertThat(solution).isNotNull(); assertThat(solution.getScore()).isNotNull(); } @@ -994,7 +994,7 @@ void solveEmptyEntityListAndEmptyValueList() { solution.setValueList(Collections.emptyList()); solution.setEntityList(Collections.emptyList()); - solution = PlannerTestUtils.solve(solverConfig, solution, false); + solution = PlannerTestUtils.solve(solverConfig, solution, true); assertThat(solution).isNotNull(); assertThat(solution.getScore()).isNotNull(); } @@ -1013,7 +1013,7 @@ void solvePinnedEntityList() { solution.setEntityList(Arrays.asList(new TestdataPinnedEntity("e1", v1, true, false), new TestdataPinnedEntity("e2", v2, false, true))); - solution = PlannerTestUtils.solve(solverConfig, solution, false); + solution = PlannerTestUtils.solve(solverConfig, solution, true); assertThat(solution).isNotNull(); assertThat(solution.getScore()).isEqualTo(SimpleScore.ZERO); } @@ -1251,7 +1251,7 @@ void solveWithCHAllowsUnassignedValuesListVariableAndTerminateInStep() { solution.setEntityList(List.of(entity)); solution.setValueList(List.of(value1)); - var bestSolution = PlannerTestUtils.solve(solverConfig, solution, false); + var bestSolution = PlannerTestUtils.solve(solverConfig, solution, true); assertThat(bestSolution.getScore()).isEqualTo(SimpleScore.of(1)); } From abf2723fcdf949d476bc16fa6343f929555ca8c9 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Thu, 21 Aug 2025 15:06:23 -0400 Subject: [PATCH 5/6] chore: review comments --- .../domain/variable/ShadowVariableUpdateHelper.java | 6 +++--- .../impl/score/director/AbstractScoreDirector.java | 13 +++++++------ .../impl/score/director/InnerScoreDirector.java | 6 +++--- .../impl/score/director/easy/EasyScoreDirector.java | 4 ++-- .../incremental/IncrementalScoreDirector.java | 4 ++-- .../stream/BavetConstraintStreamScoreDirector.java | 6 +++--- .../core/impl/solver/DefaultSolutionManager.java | 9 +++++---- .../solver/core/impl/solver/scope/SolverScope.java | 5 +++-- .../incremental/IncrementalScoreDirectorTest.java | 5 +++++ .../score/stream/bavet/BavetRegressionTest.java | 3 ++- .../core/impl/solver/MoveAssertScoreDirector.java | 4 ++-- .../score/stream/AbstractConstraintAssertion.java | 3 ++- ...ShadowVariableAwareMultiConstraintAssertion.java | 1 - ...hadowVariableAwareSingleConstraintAssertion.java | 1 - 14 files changed, 39 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java index 83f715b63c..163ad7156e 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java @@ -87,7 +87,7 @@ public void updateShadowVariables(Solution_ solution) { entityClassSet.toArray(new Class[0])); try (var scoreDirector = new InternalScoreDirector.Builder<>(solutionDescriptor).build()) { // When we have a solution, we can reuse the logic from VariableListenerSupport to update all variable types - scoreDirector.setWorkingSolution(solution); + scoreDirector.setWorkingSolutionWithoutUpdatingShadows(solution); scoreDirector.forceTriggerVariableListeners(); } } @@ -361,8 +361,8 @@ private InternalScoreDirector(Builder builder) { } @Override - public void setWorkingSolution(Solution_ workingSolution) { - super.setWorkingSolution(workingSolution, ignore -> { + public void setWorkingSolutionWithoutUpdatingShadows(Solution_ workingSolution) { + super.setWorkingSolutionWithoutUpdatingShadows(workingSolution, ignore -> { }); } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java index 5f9f1d32ed..d280f286e2 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java @@ -54,7 +54,8 @@ *

    * Implementation note: Extending classes should follow these guidelines: *

      - *
    • {@link #setWorkingSolution(Object)} should delegate to {@link #setWorkingSolution(Object, Consumer)}
      • + *
    • {@link #setWorkingSolutionWithoutUpdatingShadows(Object)} should delegate to + * {@link #setWorkingSolutionWithoutUpdatingShadows(Object, Consumer)}
      • *
    • before* method: last statement should be a call to the super method
      • *
    • after* method: first statement should be a call to the super method
      • *
    @@ -228,7 +229,7 @@ public MoveDirector getMoveDirector() { * @param workingSolution the working solution to set * @param entityAndFactVisitor maybe null; a function to apply to all problem facts and problem entities */ - protected void setWorkingSolution(Solution_ workingSolution, Consumer entityAndFactVisitor) { + protected void setWorkingSolutionWithoutUpdatingShadows(Solution_ workingSolution, Consumer entityAndFactVisitor) { this.workingSolution = requireNonNull(workingSolution); var solutionDescriptor = getSolutionDescriptor(); @@ -269,13 +270,13 @@ protected void setWorkingSolution(Solution_ workingSolution, Consumer en /** * Note: Initial Solution may have stale shadow variables! * - * @param initialSolution the initial solution + * @param workingSolution the initial solution */ @Override - public void setInitialSolution(Solution_ initialSolution) { + public final void setWorkingSolution(Solution_ workingSolution) { var originalShouldAssert = expectShadowVariablesInCorrectState; expectShadowVariablesInCorrectState = false; - setWorkingSolution(initialSolution); + setWorkingSolutionWithoutUpdatingShadows(workingSolution); forceTriggerVariableListeners(); expectShadowVariablesInCorrectState = originalShouldAssert; } @@ -419,7 +420,7 @@ public InnerScoreDirector createChildThreadScoreDirector(Chil .withLookUpEnabled(true) .withConstraintMatchPolicy(constraintMatchPolicy) .buildDerived(); - childThreadScoreDirector.setWorkingSolution(cloneWorkingSolution()); + childThreadScoreDirector.setWorkingSolutionWithoutUpdatingShadows(cloneWorkingSolution()); return childThreadScoreDirector; } else { throw new IllegalStateException("The childThreadType (" + childThreadType + ") is not implemented."); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java index 963db58c54..4080014e07 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java @@ -87,9 +87,9 @@ static > ConstraintAnalysis getConstraintAn * The {@link PlanningSolution working solution} must never be the same instance as the * {@link PlanningSolution initial solution}, it should be a clone with all shadow variables updated. * - * @param initialSolution never null + * @param workingSolution never null */ - void setInitialSolution(Solution_ initialSolution); + void setWorkingSolution(Solution_ workingSolution); /** * The {@link PlanningSolution working solution} must never be the same instance as the @@ -97,7 +97,7 @@ static > ConstraintAnalysis getConstraintAn * * @param workingSolution never null */ - void setWorkingSolution(Solution_ workingSolution); + void setWorkingSolutionWithoutUpdatingShadows(Solution_ workingSolution); /** * Different phases may need different move repositories, diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirector.java index e5afb530ab..46e7a0b552 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/easy/EasyScoreDirector.java @@ -52,8 +52,8 @@ public InnerScore calculateScore() { } @Override - public void setWorkingSolution(Solution_ workingSolution) { - super.setWorkingSolution(workingSolution, null); + public void setWorkingSolutionWithoutUpdatingShadows(Solution_ workingSolution) { + super.setWorkingSolutionWithoutUpdatingShadows(workingSolution, null); } /** diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirector.java index eded9e92ec..9ef3a0e584 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirector.java @@ -55,8 +55,8 @@ public IncrementalScoreCalculator getIncrementalScoreCalculat // ************************************************************************ @Override - public void setWorkingSolution(Solution_ workingSolution) { - super.setWorkingSolution(workingSolution, null); + public void setWorkingSolutionWithoutUpdatingShadows(Solution_ workingSolution) { + super.setWorkingSolutionWithoutUpdatingShadows(workingSolution, null); if (incrementalScoreCalculator instanceof ConstraintMatchAwareIncrementalScoreCalculator) { ((ConstraintMatchAwareIncrementalScoreCalculator) incrementalScoreCalculator) .resetWorkingSolution(workingSolution, getConstraintMatchPolicy().isEnabled()); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/stream/BavetConstraintStreamScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/stream/BavetConstraintStreamScoreDirector.java index 48e931d238..64f99dfcb8 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/stream/BavetConstraintStreamScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/stream/BavetConstraintStreamScoreDirector.java @@ -53,9 +53,9 @@ public void clearShadowVariablesListenerQueue() { } @Override - public void setWorkingSolution(Solution_ workingSolution) { + public void setWorkingSolutionWithoutUpdatingShadows(Solution_ workingSolution) { session = scoreDirectorFactory.newSession(workingSolution, constraintMatchPolicy, derived); - super.setWorkingSolution(workingSolution, session::insert); + super.setWorkingSolutionWithoutUpdatingShadows(workingSolution, session::insert); } @Override @@ -179,7 +179,7 @@ public void afterProblemFactRemoved(Object problemFact) { /** * Exposed for debugging purposes, so that we can hook into it from tests and while reproducing issues. * - * @return null before first {@link #setWorkingSolution(Object)} or after {@link #close()}. + * @return null before first {@link #setWorkingSolutionWithoutUpdatingShadows(Object)} or after {@link #close()}. */ @SuppressWarnings("unused") public BavetConstraintSession getSession() { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java index 73bb531043..ff5be61345 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java @@ -71,15 +71,16 @@ private Result_ callScoreDirector(Solution_ solution, .withExpectShadowVariablesInCorrectState(!isShadowVariableUpdateEnabled) .build()) { nonNullSolution = cloneSolution ? scoreDirector.cloneSolution(nonNullSolution) : nonNullSolution; - scoreDirector.setWorkingSolution(nonNullSolution); + if (isShadowVariableUpdateEnabled) { + scoreDirector.setWorkingSolution(nonNullSolution); + } else { + scoreDirector.setWorkingSolutionWithoutUpdatingShadows(nonNullSolution); + } if (constraintMatchPolicy.isEnabled() && !scoreDirector.getConstraintMatchPolicy().isEnabled()) { throw new IllegalStateException(""" Requested constraint matching but score director doesn't support it. Maybe use Constraint Streams instead of Easy or Incremental score calculator?"""); } - if (isShadowVariableUpdateEnabled) { - scoreDirector.forceTriggerVariableListeners(); - } if (solutionUpdatePolicy.isScoreUpdateEnabled()) { scoreDirector.calculateScore(); } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java index 2e7f975c30..7d09176595 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java @@ -332,12 +332,13 @@ public long getMoveEvaluationSpeed() { public void setWorkingSolutionFromBestSolution() { // The workingSolution must never be the same instance as the bestSolution. - scoreDirector.setWorkingSolution(scoreDirector.cloneSolution(getBestSolution())); + // The bestSolution is set by the Solver, and thus have updated shadows + scoreDirector.setWorkingSolutionWithoutUpdatingShadows(scoreDirector.cloneSolution(getBestSolution())); } public void setInitialSolution(Solution_ initialSolution) { // The workingSolution must never be the same instance as the bestSolution. - scoreDirector.setInitialSolution(scoreDirector.cloneSolution(initialSolution)); + scoreDirector.setWorkingSolution(scoreDirector.cloneSolution(initialSolution)); // Set the best solution to the solution with shadow variable updated! setBestSolution(scoreDirector.cloneSolution(scoreDirector.getWorkingSolution())); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorTest.java index 2dbda45f92..0b4fc40149 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorTest.java @@ -73,6 +73,11 @@ void variableListener() { inOrder.verify(incrementalScoreCalculator, times(1)).afterVariableChanged(a3, "chainedObject"); inOrder.verify(incrementalScoreCalculator, times(1)).beforeVariableChanged(b1, "nextEntity"); inOrder.verify(incrementalScoreCalculator, times(1)).afterVariableChanged(b1, "nextEntity"); + // Note: Anchor was not updated with scoreDirector.setWorkingSolutionWithoutUpdatingShadows, despite the fact a change is expected + // In scoreDirector.setWorkingSolutionWithoutUpdatingShadows, a3.anchor is null and should be changed to b0 (but no event is triggered; a3.anchor remained null!). + // in scoreDirector.setWorkingSolution, a3.anchor is a0 and is changed to b0. + inOrder.verify(incrementalScoreCalculator, times(1)).beforeVariableChanged(a3, "anchor"); + inOrder.verify(incrementalScoreCalculator, times(1)).afterVariableChanged(a3, "anchor"); inOrder.verifyNoMoreInteractions(); } } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetRegressionTest.java b/core/src/test/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetRegressionTest.java index a4aae82671..7dc7debda7 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetRegressionTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetRegressionTest.java @@ -534,7 +534,8 @@ void clearEvents() { .asConstraint(TEST_CONSTRAINT_NAME) }); var solution = TestdataListMultipleShadowVariableSolution.generateSolution(2, 1); - scoreDirector.setWorkingSolution(solution); + // We don't want to update shadows for this test! + scoreDirector.setWorkingSolutionWithoutUpdatingShadows(solution); scoreDirector.clearShadowVariablesListenerQueue(); assertThat(solution.getValueList().stream().allMatch(v -> v.getListenerValue() == 0)) .isTrue(); // zero if it is null diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirector.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirector.java index b43a74c3c3..c29ca3a68b 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirector.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirector.java @@ -26,8 +26,8 @@ private MoveAssertScoreDirector(Builder builder, boolean isDe } @Override - public void setWorkingSolution(Solution_ workingSolution) { - super.setWorkingSolution(workingSolution, ignored -> { + public void setWorkingSolutionWithoutUpdatingShadows(Solution_ workingSolution) { + super.setWorkingSolutionWithoutUpdatingShadows(workingSolution, ignored -> { }); } diff --git a/test/src/main/java/ai/timefold/solver/test/impl/score/stream/AbstractConstraintAssertion.java b/test/src/main/java/ai/timefold/solver/test/impl/score/stream/AbstractConstraintAssertion.java index 5fd6aa1efe..d955e71d11 100644 --- a/test/src/main/java/ai/timefold/solver/test/impl/score/stream/AbstractConstraintAssertion.java +++ b/test/src/main/java/ai/timefold/solver/test/impl/score/stream/AbstractConstraintAssertion.java @@ -38,7 +38,8 @@ void ensureInitialized() { try (var scoreDirector = scoreDirectorFactory.createScoreDirectorBuilder() .withConstraintMatchPolicy(ConstraintMatchPolicy.ENABLED) .buildDerived()) { - scoreDirector.setWorkingSolution(getSolution()); + // Users use settingAllShadowVariables to set shadow variables + scoreDirector.setWorkingSolutionWithoutUpdatingShadows(getSolution()); // When models include custom listeners, // the notification queue may no longer be empty // because the shadow variable might be linked to a source diff --git a/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareMultiConstraintAssertion.java b/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareMultiConstraintAssertion.java index 601c2df04d..b5804c063b 100644 --- a/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareMultiConstraintAssertion.java +++ b/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareMultiConstraintAssertion.java @@ -32,7 +32,6 @@ public MultiConstraintAssertion settingAllShadowVariables() { .withConstraintMatchPolicy(ConstraintMatchPolicy.ENABLED) .buildDerived()) { scoreDirector.setWorkingSolution(solution); - scoreDirector.forceTriggerVariableListeners(); update(scoreDirector.calculateScore(), scoreDirector.getConstraintMatchTotalMap(), scoreDirector.getIndictmentMap()); toggleInitialized(); diff --git a/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareSingleConstraintAssertion.java b/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareSingleConstraintAssertion.java index 2542d802f8..6c8a6f17ab 100644 --- a/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareSingleConstraintAssertion.java +++ b/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareSingleConstraintAssertion.java @@ -29,7 +29,6 @@ public SingleConstraintAssertion settingAllShadowVariables() { .withConstraintMatchPolicy(ConstraintMatchPolicy.ENABLED) .buildDerived()) { scoreDirector.setWorkingSolution(solution); - scoreDirector.forceTriggerVariableListeners(); update(scoreDirector.calculateScore(), scoreDirector.getConstraintMatchTotalMap(), scoreDirector.getIndictmentMap()); toggleInitialized(); From 4104aad3a115fa62dae7da61c5aaccca1fd9fc7b Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Fri, 22 Aug 2025 15:31:11 -0400 Subject: [PATCH 6/6] chore: review comments --- .../domain/variable/ShadowVariableUpdateHelper.java | 3 +-- .../core/impl/score/director/InnerScoreDirector.java | 12 ++++++------ .../solver/core/impl/solver/scope/SolverScope.java | 2 +- .../incremental/IncrementalScoreDirectorTest.java | 3 --- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java index 163ad7156e..7238823175 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java @@ -87,8 +87,7 @@ public void updateShadowVariables(Solution_ solution) { entityClassSet.toArray(new Class[0])); try (var scoreDirector = new InternalScoreDirector.Builder<>(solutionDescriptor).build()) { // When we have a solution, we can reuse the logic from VariableListenerSupport to update all variable types - scoreDirector.setWorkingSolutionWithoutUpdatingShadows(solution); - scoreDirector.forceTriggerVariableListeners(); + scoreDirector.setWorkingSolution(solution); } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java index 4080014e07..b5e9361c84 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java @@ -84,18 +84,18 @@ static > ConstraintAnalysis getConstraintAn } /** - * The {@link PlanningSolution working solution} must never be the same instance as the - * {@link PlanningSolution initial solution}, it should be a clone with all shadow variables updated. + * Sets the {@link PlanningSolution working solution} of the {@link ScoreDirector} + * to the given {@link PlanningSolution solution}, and force updates all the shadow variables on the given solution. * - * @param workingSolution never null + * @param workingSolution never null, must never be the same instance as the best solution. */ void setWorkingSolution(Solution_ workingSolution); /** - * The {@link PlanningSolution working solution} must never be the same instance as the - * {@link PlanningSolution best solution}, it should be a (un)changed clone. + * Sets the {@link PlanningSolution working solution} of the {@link ScoreDirector} + * to the given {@link PlanningSolution solution}, without updating any of the shadow variables on the given solution. * - * @param workingSolution never null + * @param workingSolution never null, must never be the same instance as the best solution. */ void setWorkingSolutionWithoutUpdatingShadows(Solution_ workingSolution); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java index 7d09176595..2827d010b6 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java @@ -340,7 +340,7 @@ public void setInitialSolution(Solution_ initialSolution) { // The workingSolution must never be the same instance as the bestSolution. scoreDirector.setWorkingSolution(scoreDirector.cloneSolution(initialSolution)); - // Set the best solution to the solution with shadow variable updated! + // Set the best solution to the solution with shadow variable updated. setBestSolution(scoreDirector.cloneSolution(scoreDirector.getWorkingSolution())); } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorTest.java index 0b4fc40149..1306ca5a4a 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/score/director/incremental/IncrementalScoreDirectorTest.java @@ -73,9 +73,6 @@ void variableListener() { inOrder.verify(incrementalScoreCalculator, times(1)).afterVariableChanged(a3, "chainedObject"); inOrder.verify(incrementalScoreCalculator, times(1)).beforeVariableChanged(b1, "nextEntity"); inOrder.verify(incrementalScoreCalculator, times(1)).afterVariableChanged(b1, "nextEntity"); - // Note: Anchor was not updated with scoreDirector.setWorkingSolutionWithoutUpdatingShadows, despite the fact a change is expected - // In scoreDirector.setWorkingSolutionWithoutUpdatingShadows, a3.anchor is null and should be changed to b0 (but no event is triggered; a3.anchor remained null!). - // in scoreDirector.setWorkingSolution, a3.anchor is a0 and is changed to b0. inOrder.verify(incrementalScoreCalculator, times(1)).beforeVariableChanged(a3, "anchor"); inOrder.verify(incrementalScoreCalculator, times(1)).afterVariableChanged(a3, "anchor"); inOrder.verifyNoMoreInteractions();