Skip to content

Commit dc97d27

Browse files
zepfredtriceo
authored andcommitted
fix: avoid reusing configuration on move factories (#1814)
(cherry picked from commit dbb9b53)
1 parent 5f1ea77 commit dc97d27

File tree

4 files changed

+86
-11
lines changed

4 files changed

+86
-11
lines changed

core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/SwapMoveSelectorFactory.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ public class SwapMoveSelectorFactory<Solution_>
3030
private static final Logger LOGGER = LoggerFactory.getLogger(SwapMoveSelectorFactory.class);
3131

3232
public SwapMoveSelectorFactory(SwapMoveSelectorConfig moveSelectorConfig) {
33-
super(moveSelectorConfig);
33+
// We copy the configuration,
34+
// as the settings may be updated during the autoconfiguration of the entity value range
35+
super(Objects.requireNonNull(moveSelectorConfig).copyConfig());
3436
}
3537

3638
@Override

core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListChangeMoveSelectorFactory.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ public class ListChangeMoveSelectorFactory<Solution_>
2929
extends AbstractMoveSelectorFactory<Solution_, ListChangeMoveSelectorConfig> {
3030

3131
public ListChangeMoveSelectorFactory(ListChangeMoveSelectorConfig moveSelectorConfig) {
32-
super(moveSelectorConfig);
32+
// We copy the configuration,
33+
// as the settings may be updated during the autoconfiguration of the entity value range
34+
super(Objects.requireNonNull(moveSelectorConfig).copyConfig());
3335
}
3436

3537
@Override

core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListSwapMoveSelectorFactory.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ public class ListSwapMoveSelectorFactory<Solution_>
2525
extends AbstractMoveSelectorFactory<Solution_, ListSwapMoveSelectorConfig> {
2626

2727
public ListSwapMoveSelectorFactory(ListSwapMoveSelectorConfig moveSelectorConfig) {
28-
super(moveSelectorConfig);
28+
// We copy the configuration,
29+
// as the settings may be updated during the autoconfiguration of the entity value range
30+
super(Objects.requireNonNull(moveSelectorConfig).copyConfig());
2931
}
3032

3133
@Override

core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java

Lines changed: 77 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2184,21 +2184,66 @@ void solveMultiEntityMoveConfigMixedModel(MoveSelectorConfig moveSelectionConfig
21842184
}
21852185
}
21862186

2187+
private static List<MoveSelectorConfig> generateMovesForBasicVar() {
2188+
var allMoveSelectionConfigList = new ArrayList<MoveSelectorConfig>();
2189+
// Shared entity selector config
2190+
var entitySelectorConfig = new EntitySelectorConfig()
2191+
.withEntityClass(TestdataAllowsUnassignedEntityProvidingEntity.class);
2192+
// Change - basic
2193+
allMoveSelectionConfigList.add(new ChangeMoveSelectorConfig().withEntitySelectorConfig(entitySelectorConfig));
2194+
// Swap - basic
2195+
allMoveSelectionConfigList.add(new SwapMoveSelectorConfig().withEntitySelectorConfig(entitySelectorConfig));
2196+
// Pillar change - basic
2197+
var pillarChangeMoveSelectorConfig = new PillarChangeMoveSelectorConfig();
2198+
var pillarChangeValueSelectorConfig = new ValueSelectorConfig().withVariableName("value");
2199+
pillarChangeMoveSelectorConfig
2200+
.withPillarSelectorConfig(new PillarSelectorConfig().withEntitySelectorConfig(entitySelectorConfig))
2201+
.withValueSelectorConfig(pillarChangeValueSelectorConfig);
2202+
allMoveSelectionConfigList.add(pillarChangeMoveSelectorConfig);
2203+
// Pilar swap - basic
2204+
allMoveSelectionConfigList.add(new PillarSwapMoveSelectorConfig().withPillarSelectorConfig(
2205+
new PillarSelectorConfig().withEntitySelectorConfig(entitySelectorConfig)));
2206+
// R&R - basic
2207+
allMoveSelectionConfigList.add(new RuinRecreateMoveSelectorConfig().withEntitySelectorConfig(entitySelectorConfig));
2208+
// Union of all moves
2209+
allMoveSelectionConfigList.add(new UnionMoveSelectorConfig(List.copyOf(allMoveSelectionConfigList)));
2210+
return allMoveSelectionConfigList;
2211+
}
2212+
21872213
@ParameterizedTest
2188-
@MethodSource("generateMovesForSingleVar")
2189-
void solveBasicVarEntityRangeModel(MoveSelectorConfig moveSelectionConfig) {
2214+
@MethodSource("generateMovesForBasicVar")
2215+
void solveBasicVarEntityRangeModelSingleLocalSearch(MoveSelectorConfig moveSelectionConfig) {
2216+
solveBasicVarEntityRangeModel(moveSelectionConfig, false);
2217+
}
2218+
2219+
@ParameterizedTest
2220+
@MethodSource("generateMovesForBasicVar")
2221+
void solveBasicVarEntityRangeModelMultipleLocalSearch(MoveSelectorConfig moveSelectionConfig) {
2222+
solveBasicVarEntityRangeModel(moveSelectionConfig, true);
2223+
}
2224+
2225+
private void solveBasicVarEntityRangeModel(MoveSelectorConfig moveSelectionConfig, boolean multipleLocalSearch) {
21902226
// Local search
21912227
var localSearchConfig = new LocalSearchPhaseConfig()
21922228
.withMoveSelectorConfig(moveSelectionConfig)
21932229
.withTerminationConfig(new TerminationConfig().withMoveCountLimit(1000L));
21942230

2231+
// Local search 2
2232+
var localSearchConfig2 = new LocalSearchPhaseConfig()
2233+
.withMoveSelectorConfig(moveSelectionConfig)
2234+
.withTerminationConfig(new TerminationConfig().withMoveCountLimit(1000L));
2235+
21952236
var solverConfig = PlannerTestUtils
21962237
.buildSolverConfig(TestdataAllowsUnassignedEntityProvidingSolution.class,
21972238
TestdataAllowsUnassignedEntityProvidingEntity.class)
21982239
.withEasyScoreCalculatorClass(TestdataAllowsUnassignedEntityProvidingScoreCalculator.class)
21992240
.withEnvironmentMode(EnvironmentMode.TRACKED_FULL_ASSERT)
22002241
.withPhases(new ConstructionHeuristicPhaseConfig(), localSearchConfig);
22012242

2243+
if (multipleLocalSearch) {
2244+
solverConfig.withPhases(new ConstructionHeuristicPhaseConfig(), localSearchConfig, localSearchConfig2);
2245+
}
2246+
22022247
var value1 = new TestdataValue("v1");
22032248
var value2 = new TestdataValue("v2");
22042249
var value3 = new TestdataValue("v3");
@@ -2225,16 +2270,21 @@ void solveBasicVarEntityRangeModel(MoveSelectorConfig moveSelectionConfig) {
22252270
private static List<MoveSelectorConfig> generateMovesForListVarEntityRangeModel() {
22262271
// Local Search
22272272
var allMoveSelectionConfigList = new ArrayList<MoveSelectorConfig>();
2273+
// Shared value selector config
2274+
var valueSelectorConfig = new ValueSelectorConfig()
2275+
.withVariableName("valueList");
22282276
// Change - list
2229-
allMoveSelectionConfigList.add(new ListChangeMoveSelectorConfig());
2277+
allMoveSelectionConfigList.add(new ListChangeMoveSelectorConfig().withValueSelectorConfig(valueSelectorConfig));
22302278
// Swap - list
2231-
allMoveSelectionConfigList.add(new ListSwapMoveSelectorConfig());
2279+
allMoveSelectionConfigList.add(new ListSwapMoveSelectorConfig().withValueSelectorConfig(valueSelectorConfig));
22322280
// Sublist change - list
2233-
allMoveSelectionConfigList.add(new SubListChangeMoveSelectorConfig());
2281+
allMoveSelectionConfigList.add(new SubListChangeMoveSelectorConfig()
2282+
.withSubListSelectorConfig(new SubListSelectorConfig().withValueSelectorConfig(valueSelectorConfig)));
22342283
// Sublist swap - list
2235-
allMoveSelectionConfigList.add(new SubListSwapMoveSelectorConfig());
2284+
allMoveSelectionConfigList.add(new SubListSwapMoveSelectorConfig()
2285+
.withSubListSelectorConfig(new SubListSelectorConfig().withValueSelectorConfig(valueSelectorConfig)));
22362286
// KOpt - list
2237-
allMoveSelectionConfigList.add(new KOptListMoveSelectorConfig());
2287+
allMoveSelectionConfigList.add(new KOptListMoveSelectorConfig().withValueSelectorConfig(valueSelectorConfig));
22382288
// R&R - list
22392289
allMoveSelectionConfigList.add(new ListRuinRecreateMoveSelectorConfig());
22402290
// Union of all moves
@@ -2244,19 +2294,38 @@ private static List<MoveSelectorConfig> generateMovesForListVarEntityRangeModel(
22442294

22452295
@ParameterizedTest
22462296
@MethodSource("generateMovesForListVarEntityRangeModel")
2247-
void solveListVarEntityRangeModel(MoveSelectorConfig moveSelectionConfig) {
2297+
void solveListVarEntityRangeModelSingleLocalSearch(MoveSelectorConfig moveSelectionConfig) {
2298+
solveListVarEntityRangeModel(moveSelectionConfig, false);
2299+
}
2300+
2301+
@ParameterizedTest
2302+
@MethodSource("generateMovesForListVarEntityRangeModel")
2303+
void solveListVarEntityRangeModelMultipleLocalSearch(MoveSelectorConfig moveSelectionConfig) {
2304+
solveListVarEntityRangeModel(moveSelectionConfig, true);
2305+
}
2306+
2307+
private void solveListVarEntityRangeModel(MoveSelectorConfig moveSelectionConfig, boolean multipleLocalSearch) {
22482308
// Local search
22492309
var localSearchConfig = new LocalSearchPhaseConfig()
22502310
.withMoveSelectorConfig(moveSelectionConfig)
22512311
.withTerminationConfig(new TerminationConfig().withMoveCountLimit(1000L));
22522312

2313+
// Local search 2
2314+
var localSearchConfig2 = new LocalSearchPhaseConfig()
2315+
.withMoveSelectorConfig(moveSelectionConfig)
2316+
.withTerminationConfig(new TerminationConfig().withMoveCountLimit(1000L));
2317+
22532318
var solverConfig = PlannerTestUtils
22542319
.buildSolverConfig(TestdataListUnassignedEntityProvidingSolution.class,
22552320
TestdataListUnassignedEntityProvidingEntity.class)
22562321
.withEnvironmentMode(EnvironmentMode.TRACKED_FULL_ASSERT)
22572322
.withEasyScoreCalculatorClass(TestdataListUnassignedEntityProvidingScoreCalculator.class)
22582323
.withPhases(new ConstructionHeuristicPhaseConfig(), localSearchConfig);
22592324

2325+
if (multipleLocalSearch) {
2326+
solverConfig.withPhases(new ConstructionHeuristicPhaseConfig(), localSearchConfig, localSearchConfig2);
2327+
}
2328+
22602329
var value1 = new TestdataValue("v1");
22612330
var value2 = new TestdataValue("v2");
22622331
var value3 = new TestdataValue("v3");

0 commit comments

Comments
 (0)