Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ static <Solution_, Score_ extends Score<Score_>, ProblemId_> ScoreManager<Soluti

/**
* Calculates the {@link Score} of a {@link PlanningSolution} and updates its {@link PlanningScore} member.
* <p>
* 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
*/
Expand All @@ -77,7 +83,7 @@ static <Solution_, Score_ extends Score<Score_>, ProblemId_> ScoreManager<Soluti
* constraints or planning entities cause that score quality.
* In case of an {@link Score#isFeasible() infeasible} solution, this can help diagnose the cause of that.
* <p>
* 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.
Expand All @@ -92,6 +98,12 @@ static <Solution_, Score_ extends Score<Score_>, ProblemId_> ScoreManager<Soluti
/**
* Calculates and retrieves {@link ConstraintMatchTotal}s and {@link Indictment}s necessary for describing the
* quality of a particular solution.
* <p>
* 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -95,7 +95,7 @@ public class EntityDescriptor<Solution_> {
PiggybackShadowVariable.class,
CustomShadowVariable.class,
CascadingUpdateShadowVariable.class,
ShadowVariableLooped.class
ShadowVariablesInconsistent.class
};

private static final Logger LOGGER = LoggerFactory.getLogger(EntityDescriptor.class);
Expand All @@ -107,7 +107,7 @@ public class EntityDescriptor<Solution_> {
private final Predicate<Object> isInitializedPredicate;
private final List<MemberAccessor> declaredPlanningPinIndexMemberAccessorList = new ArrayList<>();
@Nullable
private ShadowVariableLoopedVariableDescriptor<Solution_> shadowVariableLoopedDescriptor;
private ShadowVariablesInconsistentVariableDescriptor<Solution_> shadowVariablesInconsistentDescriptor;

private Predicate<Object> hasNoNullVariablesBasicVar;
private Predicate<Object> hasNoNullVariablesListVar;
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -678,8 +678,8 @@ public GenuineVariableDescriptor<Solution_> getGenuineVariableDescriptor(String
return effectiveGenuineVariableDescriptorMap.get(variableName);
}

public @Nullable ShadowVariableLoopedVariableDescriptor<Solution_> getShadowVariableLoopedDescriptor() {
return shadowVariableLoopedDescriptor;
public @Nullable ShadowVariablesInconsistentVariableDescriptor<Solution_> getShadowVariablesInconsistentDescriptor() {
return shadowVariablesInconsistentDescriptor;
}

public boolean hasBothGenuineListAndBasicVariables() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ public void updateShadowVariables(Solution_ solution) {
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.forceTriggerVariableListeners();
}
}

Expand Down Expand Up @@ -361,8 +360,8 @@ private InternalScoreDirector(Builder<Solution_, Score_> builder) {
}

@Override
public void setWorkingSolution(Solution_ workingSolution) {
super.setWorkingSolution(workingSolution, ignore -> {
public void setWorkingSolutionWithoutUpdatingShadows(Solution_ workingSolution) {
super.setWorkingSolutionWithoutUpdatingShadows(workingSolution, ignore -> {
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ private void initializeChangeQueue(BitSet changed) {
changed.clear();
}

private boolean updateEntityShadowVariables(GraphNode<Solution_> entityVariable, boolean isVariableLooped) {
private boolean updateEntityShadowVariables(GraphNode<Solution_> entityVariable, boolean isVariableInconsistent) {
var entity = entityVariable.entity();
var shadowVariableReferences = entityVariable.variableReferences();
var loopDescriptor = shadowVariableReferences.get(0).shadowVariableLoopedDescriptor();
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();
Expand All @@ -136,32 +136,32 @@ private boolean updateEntityShadowVariables(GraphNode<Solution_> 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;
}

private boolean updateLoopedStatusOfEntity(Object entity, int entityId,
ShadowVariableLoopedVariableDescriptor<Solution_> loopDescriptor) {
var oldLooped = (boolean) loopDescriptor.getValue(entity);
var isEntityLooped = loopedTracker.isEntityLooped(graph, entityId, oldLooped);
ShadowVariablesInconsistentVariableDescriptor<Solution_> loopDescriptor) {
var oldLooped = (Boolean) loopDescriptor.getValue(entity);
var isEntityLooped = loopedTracker.isEntityInconsistent(graph, entityId, oldLooped);
if (!Objects.equals(oldLooped, isEntityLooped)) {
changeShadowVariableAndNotify(loopDescriptor, entity, isEntityLooped);
}
// We return true if the entity's loop status changed at any point;
// 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ public List<VariableUpdaterInfo<Solution_>> getUpdatersForEntityVariable(Object
declarativeShadowVariableDescriptor.getVariableMetaModel(),
updaterKey,
declarativeShadowVariableDescriptor,
declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(),
declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariablesInconsistentDescriptor(),
declarativeShadowVariableDescriptor.getMemberAccessor(),
declarativeShadowVariableDescriptor.getCalculator()::executeGetter);
if (declarativeShadowVariableDescriptor.getAlignmentKeyMap() != null) {
Expand Down Expand Up @@ -354,7 +354,8 @@ private static <Solution_> VariableReferenceGraph buildArbitrarySingleEntityGrap
variableId,
variableIdToUpdaters.getNextId(),
declarativeShadowVariableDescriptor,
declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(),
declarativeShadowVariableDescriptor.getEntityDescriptor()
.getShadowVariablesInconsistentDescriptor(),
declarativeShadowVariableDescriptor.getMemberAccessor(),
declarativeShadowVariableDescriptor.getCalculator()::executeGetter)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
}
Expand All @@ -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) {
Expand All @@ -60,7 +62,7 @@ public LoopedStatus status(int node) {
}

public void clear() {
Arrays.fill(entityLoopedStatusChanged, false);
Arrays.fill(entityInconsistentStatusChanged, false);
looped.clear();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 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?
""".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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Solution_> extends ShadowVariableDescriptor<Solution_> {
public ShadowVariableLoopedVariableDescriptor(int ordinal,
public class ShadowVariablesInconsistentVariableDescriptor<Solution_> extends ShadowVariableDescriptor<Solution_> {
public ShadowVariablesInconsistentVariableDescriptor(int ordinal,
EntityDescriptor<Solution_> entityDescriptor,
MemberAccessor variableMemberAccessor) {
super(ordinal, entityDescriptor, variableMemberAccessor);
Expand All @@ -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()));
}
}

Expand Down
Loading
Loading