Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure entity placer is not shared in ruin and recreate moves #1320

Merged
merged 14 commits into from
Jan 15, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ protected ConstructionHeuristicPhaseScope<Solution_> buildPhaseScope(SolverScope
return new ConstructionHeuristicPhaseScope<>(solverScope, phaseIndex);
}

private void doStep(ConstructionHeuristicStepScope<Solution_> stepScope) {
protected void doStep(ConstructionHeuristicStepScope<Solution_> stepScope) {
var step = stepScope.getStep();
step.execute(stepScope.getMoveDirector());
predictWorkingStepScore(stepScope, step);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package ai.timefold.solver.core.impl.constructionheuristic.placer;

import ai.timefold.solver.core.impl.heuristic.HeuristicConfigPolicy;
import ai.timefold.solver.core.impl.phase.event.PhaseLifecycleSupport;
import ai.timefold.solver.core.impl.phase.scope.AbstractPhaseScope;
import ai.timefold.solver.core.impl.phase.scope.AbstractStepScope;
Expand All @@ -17,8 +18,24 @@ public abstract class AbstractEntityPlacer<Solution_> implements EntityPlacer<So

protected final transient Logger logger = LoggerFactory.getLogger(getClass());

protected final EntityPlacerFactory<Solution_> factory;
protected final HeuristicConfigPolicy<Solution_> configPolicy;

protected PhaseLifecycleSupport<Solution_> phaseLifecycleSupport = new PhaseLifecycleSupport<>();

AbstractEntityPlacer(EntityPlacerFactory<Solution_> factory, HeuristicConfigPolicy<Solution_> configPolicy) {
this.factory = factory;
this.configPolicy = configPolicy;
}

@Override
public EntityPlacer<Solution_> copy() {
if (factory == null || configPolicy == null) {
throw new IllegalStateException("The entity placer cannot be copied.");
}
return factory.buildEntityPlacer(configPolicy.copyConfigPolicy());
}

@Override
public void solvingStarted(SolverScope<Solution_> solverScope) {
phaseLifecycleSupport.fireSolvingStarted(solverScope);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ public interface EntityPlacer<Solution_> extends Iterable<Placement<Solution_>>,

EntityPlacer<Solution_> rebuildWithFilter(SelectionFilter<Solution_, Object> filter);

EntityPlacer<Solution_> copy();
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.Iterator;

import ai.timefold.solver.core.impl.heuristic.HeuristicConfigPolicy;
import ai.timefold.solver.core.impl.heuristic.move.Move;
import ai.timefold.solver.core.impl.heuristic.selector.common.decorator.SelectionFilter;
import ai.timefold.solver.core.impl.heuristic.selector.common.iterator.UpcomingSelectionIterator;
Expand All @@ -13,7 +14,9 @@ public class PooledEntityPlacer<Solution_> extends AbstractEntityPlacer<Solution

protected final MoveSelector<Solution_> moveSelector;

public PooledEntityPlacer(MoveSelector<Solution_> moveSelector) {
public PooledEntityPlacer(EntityPlacerFactory<Solution_> factory, HeuristicConfigPolicy<Solution_> configPolicy,
MoveSelector<Solution_> moveSelector) {
super(factory, configPolicy);
this.moveSelector = moveSelector;
phaseLifecycleSupport.addEventListener(moveSelector);
}
Expand All @@ -25,7 +28,7 @@ public Iterator<Placement<Solution_>> iterator() {

@Override
public EntityPlacer<Solution_> rebuildWithFilter(SelectionFilter<Solution_, Object> filter) {
return new PooledEntityPlacer<>(FilteringMoveSelector.of(moveSelector, filter::accept));
return new PooledEntityPlacer<>(factory, configPolicy, FilteringMoveSelector.of(moveSelector, filter::accept));
}

private class PooledEntityPlacingIterator extends UpcomingSelectionIterator<Placement<Solution_>> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public PooledEntityPlacer<Solution_> buildEntityPlacer(HeuristicConfigPolicy<Sol

MoveSelector<Solution_> moveSelector = MoveSelectorFactory.<Solution_> create(moveSelectorConfig_)
.buildMoveSelector(configPolicy, SelectionCacheType.JUST_IN_TIME, SelectionOrder.ORIGINAL, false);
return new PooledEntityPlacer<>(moveSelector);
return new PooledEntityPlacer<>(this, configPolicy, moveSelector);
}

private MoveSelectorConfig buildMoveSelectorConfig(HeuristicConfigPolicy<Solution_> configPolicy) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.Iterator;
import java.util.List;

import ai.timefold.solver.core.impl.heuristic.HeuristicConfigPolicy;
import ai.timefold.solver.core.impl.heuristic.move.Move;
import ai.timefold.solver.core.impl.heuristic.selector.common.decorator.SelectionFilter;
import ai.timefold.solver.core.impl.heuristic.selector.common.iterator.UpcomingSelectionIterator;
Expand All @@ -17,7 +18,9 @@ public class QueuedEntityPlacer<Solution_> extends AbstractEntityPlacer<Solution
protected final EntitySelector<Solution_> entitySelector;
protected final List<MoveSelector<Solution_>> moveSelectorList;

public QueuedEntityPlacer(EntitySelector<Solution_> entitySelector, List<MoveSelector<Solution_>> moveSelectorList) {
public QueuedEntityPlacer(EntityPlacerFactory<Solution_> factory, HeuristicConfigPolicy<Solution_> configPolicy,
EntitySelector<Solution_> entitySelector, List<MoveSelector<Solution_>> moveSelectorList) {
super(factory, configPolicy);
this.entitySelector = entitySelector;
this.moveSelectorList = moveSelectorList;
phaseLifecycleSupport.addEventListener(entitySelector);
Expand All @@ -33,7 +36,8 @@ public Iterator<Placement<Solution_>> iterator() {

@Override
public EntityPlacer<Solution_> rebuildWithFilter(SelectionFilter<Solution_, Object> filter) {
return new QueuedEntityPlacer<>(FilteringEntitySelector.of(entitySelector, filter), moveSelectorList);
return new QueuedEntityPlacer<>(factory, configPolicy, FilteringEntitySelector.of(entitySelector, filter),
moveSelectorList);
}

private class QueuedEntityPlacingIterator extends UpcomingSelectionIterator<Placement<Solution_>> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public QueuedEntityPlacer<Solution_> buildEntityPlacer(HeuristicConfigPolicy<Sol
.buildMoveSelector(configPolicy, SelectionCacheType.JUST_IN_TIME, SelectionOrder.ORIGINAL, false);
moveSelectorList.add(moveSelector);
}
return new QueuedEntityPlacer<>(entitySelector, moveSelectorList);
return new QueuedEntityPlacer<>(this, configPolicy, entitySelector, moveSelectorList);
}

@SuppressWarnings("rawtypes")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.util.Collections;
import java.util.Iterator;

import ai.timefold.solver.core.impl.heuristic.HeuristicConfigPolicy;
import ai.timefold.solver.core.impl.heuristic.selector.common.decorator.SelectionFilter;
import ai.timefold.solver.core.impl.heuristic.selector.common.iterator.UpcomingSelectionIterator;
import ai.timefold.solver.core.impl.heuristic.selector.move.MoveSelector;
Expand All @@ -16,8 +17,9 @@ public class QueuedValuePlacer<Solution_> extends AbstractEntityPlacer<Solution_
protected final EntityIndependentValueSelector<Solution_> valueSelector;
protected final MoveSelector<Solution_> moveSelector;

public QueuedValuePlacer(EntityIndependentValueSelector<Solution_> valueSelector,
MoveSelector<Solution_> moveSelector) {
public QueuedValuePlacer(EntityPlacerFactory<Solution_> factory, HeuristicConfigPolicy<Solution_> configPolicy,
EntityIndependentValueSelector<Solution_> valueSelector, MoveSelector<Solution_> moveSelector) {
super(factory, configPolicy);
this.valueSelector = valueSelector;
this.moveSelector = moveSelector;
phaseLifecycleSupport.addEventListener(valueSelector);
Expand Down Expand Up @@ -59,7 +61,7 @@ protected Placement<Solution_> createUpcomingSelection() {

@Override
public EntityPlacer<Solution_> rebuildWithFilter(SelectionFilter<Solution_, Object> filter) {
return new QueuedValuePlacer<>(
return new QueuedValuePlacer<>(factory, configPolicy,
(EntityIndependentFilteringValueSelector<Solution_>) FilteringValueSelector.of(valueSelector, filter),
moveSelector);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ public QueuedValuePlacer<Solution_> buildEntityPlacer(HeuristicConfigPolicy<Solu
+ " Check your @" + ValueRangeProvider.class.getSimpleName() + " annotations.");

}
return new QueuedValuePlacer<>((EntityIndependentValueSelector<Solution_>) valueSelector, moveSelector);
return new QueuedValuePlacer<>(this, configPolicy, (EntityIndependentValueSelector<Solution_>) valueSelector,
moveSelector);
}

private ValueSelectorConfig buildValueSelectorConfig(HeuristicConfigPolicy<Solution_> configPolicy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ public Builder<Solution_> cloneBuilder() {
.withLogIndentation(logIndentation);
}

public HeuristicConfigPolicy<Solution_> copyConfigPolicy() {
return cloneBuilder()
.withEntitySorterManner(entitySorterManner)
.withValueSorterManner(valueSorterManner)
.withReinitializeVariableFilterEnabled(reinitializeVariableFilterEnabled)
.withInitializedChainedValueFilterEnabled(initializedChainedValueFilterEnabled)
.withUnassignedValuesAllowed(unassignedValuesAllowed)
.build();
}

public HeuristicConfigPolicy<Solution_> createPhaseConfigPolicy() {
return cloneBuilder().build();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,28 @@
package ai.timefold.solver.core.impl.heuristic.selector.move.generic;

import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

import ai.timefold.solver.core.impl.constructionheuristic.ConstructionHeuristicPhase;
import ai.timefold.solver.core.impl.constructionheuristic.DefaultConstructionHeuristicPhase;
import ai.timefold.solver.core.impl.constructionheuristic.scope.ConstructionHeuristicPhaseScope;
import ai.timefold.solver.core.impl.constructionheuristic.scope.ConstructionHeuristicStepScope;
import ai.timefold.solver.core.impl.solver.scope.SolverScope;

final class RuinRecreateConstructionHeuristicPhase<Solution_>
public final class RuinRecreateConstructionHeuristicPhase<Solution_>
extends DefaultConstructionHeuristicPhase<Solution_>
implements ConstructionHeuristicPhase<Solution_> {

private final Set<Object> elementsToRuinSet;
// Store the original value list of elements that are not included in the initial list of ruined elements
private final Map<Object, List<Object>> missingUpdatedElementsMap;

RuinRecreateConstructionHeuristicPhase(RuinRecreateConstructionHeuristicPhaseBuilder<Solution_> builder) {
super(builder);
this.elementsToRuinSet = builder.elementsToRuin;
this.missingUpdatedElementsMap = new IdentityHashMap<>();
}

@Override
Expand All @@ -28,4 +40,24 @@ public String getPhaseTypeString() {
return "Ruin & Recreate Construction Heuristics";
}

@Override
protected void doStep(ConstructionHeuristicStepScope<Solution_> stepScope) {
if (elementsToRuinSet != null) {
var listVariableDescriptor = stepScope.getPhaseScope().getSolverScope().getSolutionDescriptor()
.getListVariableDescriptor();
var entity = stepScope.getStep().extractPlanningEntities().iterator().next();
if (!elementsToRuinSet.contains(entity)) {
// Sometimes, the list of elements to be ruined does not include new destinations selected by the CH.
// In these cases, we need to record the element list before making any move changes
// so that it can be referenced to restore the solution to its original state when undoing changes.
missingUpdatedElementsMap.computeIfAbsent(entity,
e -> List.copyOf(listVariableDescriptor.getValue(e)));
}
}
super.doStep(stepScope);
}

public Map<Object, List<Object>> getMissingUpdatedElementsMap() {
return missingUpdatedElementsMap;
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package ai.timefold.solver.core.impl.heuristic.selector.move.generic;

import java.util.List;
import java.util.Set;

import ai.timefold.solver.core.config.constructionheuristic.ConstructionHeuristicPhaseConfig;
import ai.timefold.solver.core.config.solver.termination.TerminationConfig;
import ai.timefold.solver.core.impl.constructionheuristic.DefaultConstructionHeuristicPhase;
import ai.timefold.solver.core.impl.constructionheuristic.decider.ConstructionHeuristicDecider;
import ai.timefold.solver.core.impl.constructionheuristic.placer.EntityPlacer;
import ai.timefold.solver.core.impl.heuristic.HeuristicConfigPolicy;
import ai.timefold.solver.core.impl.score.director.InnerScoreDirector;
import ai.timefold.solver.core.impl.solver.termination.Termination;
import ai.timefold.solver.core.impl.solver.termination.TerminationFactory;

Expand All @@ -24,29 +26,68 @@ public static <Solution_> RuinRecreateConstructionHeuristicPhaseBuilder<Solution
HeuristicConfigPolicy<Solution_> solverConfigPolicy, ConstructionHeuristicPhaseConfig constructionHeuristicConfig) {
var constructionHeuristicPhaseFactory =
new RuinRecreateConstructionHeuristicPhaseFactory<Solution_>(constructionHeuristicConfig);
return (RuinRecreateConstructionHeuristicPhaseBuilder<Solution_>) constructionHeuristicPhaseFactory.getBuilder(0, false,
var builder = (RuinRecreateConstructionHeuristicPhaseBuilder<Solution_>) constructionHeuristicPhaseFactory.getBuilder(0,
false,
solverConfigPolicy, TerminationFactory.<Solution_> create(new TerminationConfig())
.buildTermination(solverConfigPolicy));
if (solverConfigPolicy.getMoveThreadCount() != null && solverConfigPolicy.getMoveThreadCount() >= 1) {
builder.multithreaded = true;
}
return builder;
}

private List<Object> elementsToRecreate;
private final HeuristicConfigPolicy<Solution_> configPolicy;
private final RuinRecreateConstructionHeuristicPhaseFactory<Solution_> constructionHeuristicPhaseFactory;
private final Termination<Solution_> phaseTermination;

Set<Object> elementsToRuin;
List<Object> elementsToRecreate;
private boolean multithreaded = false;

RuinRecreateConstructionHeuristicPhaseBuilder(Termination<Solution_> phaseTermination,
EntityPlacer<Solution_> entityPlacer, ConstructionHeuristicDecider<Solution_> decider) {
RuinRecreateConstructionHeuristicPhaseBuilder(HeuristicConfigPolicy<Solution_> configPolicy,
RuinRecreateConstructionHeuristicPhaseFactory<Solution_> constructionHeuristicPhaseFactory,
Termination<Solution_> phaseTermination, EntityPlacer<Solution_> entityPlacer,
ConstructionHeuristicDecider<Solution_> decider) {
super(0, false, "", phaseTermination, entityPlacer, decider);
this.configPolicy = configPolicy;
this.constructionHeuristicPhaseFactory = constructionHeuristicPhaseFactory;
this.phaseTermination = phaseTermination;
}

/**
* In a multithreaded environment, the builder will be shared among all moves and threads.
* Consequently, the list {@code elementsToRecreate} used by {@code getEntityPlacer} or the {@code decider},
* will be shared between the main and move threads.
* This sharing can lead to race conditions.
* The method creates a new copy of the builder and the decider to avoid race conditions.
*/
public RuinRecreateConstructionHeuristicPhaseBuilder<Solution_>
ensureThreadSafe(InnerScoreDirector<Solution_, ?> scoreDirector) {
if (multithreaded && scoreDirector.isDerived()) {
return new RuinRecreateConstructionHeuristicPhaseBuilder<>(configPolicy, constructionHeuristicPhaseFactory,
phaseTermination, super.getEntityPlacer().copy(),
constructionHeuristicPhaseFactory.buildDecider(configPolicy, phaseTermination));
}
return this;
}

public RuinRecreateConstructionHeuristicPhaseBuilder<Solution_> withElementsToRecreate(List<Object> elements) {
this.elementsToRecreate = elements;
return this;
}

public RuinRecreateConstructionHeuristicPhaseBuilder<Solution_> withElementsToRuin(Set<Object> elements) {
this.elementsToRuin = elements;
return this;
}

@Override
public EntityPlacer<Solution_> getEntityPlacer() {
var placer = super.getEntityPlacer();
if (elementsToRecreate == null || elementsToRecreate.isEmpty()) {
return super.getEntityPlacer();
return placer;
}
return super.getEntityPlacer().rebuildWithFilter((scoreDirector, selection) -> {
return placer.rebuildWithFilter((scoreDirector, selection) -> {
for (var element : elementsToRecreate) {
if (selection == element) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ protected DefaultConstructionHeuristicPhaseBuilder<Solution_> createBuilder(
Termination<Solution_> solverTermination, int phaseIndex, boolean triggerFirstInitializedSolutionEvent,
EntityPlacer<Solution_> entityPlacer) {
var phaseTermination = new PhaseToSolverTerminationBridge<>(new BasicPlumbingTermination<Solution_>(false));
return new RuinRecreateConstructionHeuristicPhaseBuilder<>(phaseTermination, entityPlacer,
return new RuinRecreateConstructionHeuristicPhaseBuilder<>(phaseConfigPolicy, this, phaseTermination, entityPlacer,
buildDecider(phaseConfigPolicy, phaseTermination));

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import ai.timefold.solver.core.impl.domain.variable.descriptor.GenuineVariableDescriptor;
import ai.timefold.solver.core.impl.heuristic.move.AbstractMove;
import ai.timefold.solver.core.impl.heuristic.move.Move;
import ai.timefold.solver.core.impl.score.director.VariableDescriptorAwareScoreDirector;
import ai.timefold.solver.core.impl.move.director.VariableChangeRecordingScoreDirector;
import ai.timefold.solver.core.impl.solver.scope.SolverScope;

public final class RuinRecreateMove<Solution_> extends AbstractMove<Solution_> {
Expand Down Expand Up @@ -38,21 +38,23 @@ public RuinRecreateMove(GenuineVariableDescriptor<Solution_> genuineVariableDesc
protected void doMoveOnGenuineVariables(ScoreDirector<Solution_> scoreDirector) {
recordedNewValues = new Object[ruinedEntityList.size()];

var castScoreDirector = (VariableDescriptorAwareScoreDirector<Solution_>) scoreDirector;
var recordingScoreDirector = (VariableChangeRecordingScoreDirector<Solution_>) scoreDirector;
for (var ruinedEntity : ruinedEntityList) {
castScoreDirector.beforeVariableChanged(genuineVariableDescriptor, ruinedEntity);
recordingScoreDirector.beforeVariableChanged(genuineVariableDescriptor, ruinedEntity);
genuineVariableDescriptor.setValue(ruinedEntity, null);
castScoreDirector.afterVariableChanged(genuineVariableDescriptor, ruinedEntity);
recordingScoreDirector.afterVariableChanged(genuineVariableDescriptor, ruinedEntity);
}
castScoreDirector.triggerVariableListeners();
recordingScoreDirector.triggerVariableListeners();

var constructionHeuristicPhase = constructionHeuristicPhaseBuilder.withElementsToRecreate(ruinedEntityList)
var constructionHeuristicPhase = constructionHeuristicPhaseBuilder
.ensureThreadSafe(recordingScoreDirector.getBacking())
.withElementsToRecreate(ruinedEntityList)
.build();
constructionHeuristicPhase.setSolver(solverScope.getSolver());
constructionHeuristicPhase.solvingStarted(solverScope);
constructionHeuristicPhase.solve(solverScope);
constructionHeuristicPhase.solvingEnded(solverScope);
castScoreDirector.triggerVariableListeners();
recordingScoreDirector.triggerVariableListeners();

for (var i = 0; i < ruinedEntityList.size(); i++) {
recordedNewValues[i] = genuineVariableDescriptor.getValue(ruinedEntityList.get(i));
Expand Down
Loading
Loading