From 5e3375f88dc98ff44e21fe14698d0e2e6e086735 Mon Sep 17 00:00:00 2001 From: Fred Date: Tue, 5 Mar 2024 07:54:47 -0300 Subject: [PATCH] fix: Lukas' comments --- .../impl/heuristic/HeuristicConfigPolicy.java | 13 +- .../DefaultLocalSearchPhaseFactory.java | 240 ++++++++++-------- .../impl/solver/DefaultSolverFactory.java | 4 +- .../HeuristicConfigPolicyTestUtils.java | 4 +- 4 files changed, 146 insertions(+), 115 deletions(-) diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicy.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicy.java index d0228ab11a..8a28f04955 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicy.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicy.java @@ -2,6 +2,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Random; import java.util.concurrent.ThreadFactory; import ai.timefold.solver.core.config.heuristic.selector.entity.EntitySorterManner; @@ -38,6 +39,7 @@ public class HeuristicConfigPolicy { private final boolean initializedChainedValueFilterEnabled; private final boolean unassignedValuesAllowed; private final Class> nearbyDistanceMeterClass; + private final Random random; private final Map> entityMimicRecorderMap = new HashMap<>(); private final Map> subListMimicRecorderMap = new HashMap<>(); @@ -58,6 +60,7 @@ private HeuristicConfigPolicy(Builder builder) { this.initializedChainedValueFilterEnabled = builder.initializedChainedValueFilterEnabled; this.unassignedValuesAllowed = builder.unassignedValuesAllowed; this.nearbyDistanceMeterClass = builder.nearbyDistanceMeterClass; + this.random = builder.random; } public EnvironmentMode getEnvironmentMode() { @@ -116,13 +119,17 @@ public Class getNearbyDistanceMeterClass() { return nearbyDistanceMeterClass; } + public Random getRandom() { + return random; + } + // ************************************************************************ // Builder methods // ************************************************************************ public Builder cloneBuilder() { return new Builder<>(environmentMode, moveThreadCount, moveThreadBufferSize, threadFactoryClass, - nearbyDistanceMeterClass, initializingScoreTrend, solutionDescriptor, classInstanceCache) + nearbyDistanceMeterClass, random, initializingScoreTrend, solutionDescriptor, classInstanceCache) .withLogIndentation(logIndentation); } @@ -223,10 +230,11 @@ public static class Builder { private boolean unassignedValuesAllowed = false; private final Class> nearbyDistanceMeterClass; + private final Random random; public Builder(EnvironmentMode environmentMode, Integer moveThreadCount, Integer moveThreadBufferSize, Class threadFactoryClass, - Class> nearbyDistanceMeterClass, + Class> nearbyDistanceMeterClass, Random random, InitializingScoreTrend initializingScoreTrend, SolutionDescriptor solutionDescriptor, ClassInstanceCache classInstanceCache) { this.environmentMode = environmentMode; @@ -234,6 +242,7 @@ public Builder(EnvironmentMode environmentMode, Integer moveThreadCount, Integer this.moveThreadBufferSize = moveThreadBufferSize; this.threadFactoryClass = threadFactoryClass; this.nearbyDistanceMeterClass = nearbyDistanceMeterClass; + this.random = random; this.initializingScoreTrend = initializingScoreTrend; this.solutionDescriptor = solutionDescriptor; this.classInstanceCache = classInstanceCache; diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseFactory.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseFactory.java index 04e09aff9f..af3ef72dba 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseFactory.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseFactory.java @@ -2,7 +2,7 @@ import java.util.Collections; import java.util.Objects; -import java.util.UUID; +import java.util.Random; import ai.timefold.solver.core.config.heuristic.selector.common.SelectionCacheType; import ai.timefold.solver.core.config.heuristic.selector.common.SelectionOrder; @@ -204,116 +204,10 @@ private UnionMoveSelectorConfig determineDefaultMoveSelectorConfig(HeuristicConf .anyMatch(v -> ((BasicVariableDescriptor) v).isChained()); var listVariableDescriptor = solutionDescriptor.getListVariableDescriptor(); if (basicVariableDescriptorList.isEmpty()) { // We only have the one list variable. - if (configPolicy.getNearbyDistanceMeterClass() == null) { - return new UnionMoveSelectorConfig() - .withMoveSelectors(new ListChangeMoveSelectorConfig(), new ListSwapMoveSelectorConfig(), - new KOptListMoveSelectorConfig()); - } else { - String changeSelectorName = "changeMoveSelector-%s".formatted(UUID.randomUUID().toString().substring(0, 8)); - String swapSelectorName = "swapMoveSelector-%s".formatted(UUID.randomUUID().toString().substring(0, 8)); - String koptSelectorName = "koptMoveSelector-%s".formatted(UUID.randomUUID().toString().substring(0, 8)); - return new UnionMoveSelectorConfig() - .withMoveSelectors(new ListChangeMoveSelectorConfig(), - new ListSwapMoveSelectorConfig(), - new ListChangeMoveSelectorConfig() - .withValueSelectorConfig(new ValueSelectorConfig() - .withId(changeSelectorName)) - .withDestinationSelectorConfig(new DestinationSelectorConfig() - .withNearbySelectionConfig(new NearbySelectionConfig() - .withOriginValueSelectorConfig(new ValueSelectorConfig() - .withMimicSelectorRef(changeSelectorName)) - .withNearbyDistanceMeterClass( - configPolicy.getNearbyDistanceMeterClass()))), - new ListSwapMoveSelectorConfig() - .withValueSelectorConfig(new ValueSelectorConfig() - .withId(swapSelectorName)) - .withSecondaryValueSelectorConfig(new ValueSelectorConfig() - .withNearbySelectionConfig(new NearbySelectionConfig() - .withOriginValueSelectorConfig(new ValueSelectorConfig() - .withMimicSelectorRef(swapSelectorName)) - .withNearbyDistanceMeterClass( - configPolicy.getNearbyDistanceMeterClass()))), - new KOptListMoveSelectorConfig() - .withOriginSelectorConfig(new ValueSelectorConfig() - .withId(koptSelectorName)) - .withValueSelectorConfig(new ValueSelectorConfig() - .withNearbySelectionConfig(new NearbySelectionConfig() - .withOriginValueSelectorConfig(new ValueSelectorConfig() - .withMimicSelectorRef(koptSelectorName)) - .withNearbyDistanceMeterClass( - configPolicy.getNearbyDistanceMeterClass())))); - } + return determineDefaultListVarMoveSelectorConfig(configPolicy); } else if (listVariableDescriptor == null) { // We only have basic variables. - if (configPolicy.getNearbyDistanceMeterClass() == null) { - if (hasChainedVariable) { - return new UnionMoveSelectorConfig() - .withMoveSelectors(new ChangeMoveSelectorConfig(), new SwapMoveSelectorConfig(), - new TailChainSwapMoveSelectorConfig()); - } else { - return new UnionMoveSelectorConfig() - .withMoveSelectors(new ChangeMoveSelectorConfig(), new SwapMoveSelectorConfig()); - } - } else { - if (hasChainedVariable) { - String changeSelectorName = "changeMoveSelector-%s".formatted(UUID.randomUUID().toString().substring(0, 8)); - String swapSelectorName = "swapMoveSelector-%s".formatted(UUID.randomUUID().toString().substring(0, 8)); - String tailChainSelectorName = - "tailChainSwapMoveSelector-%s".formatted(UUID.randomUUID().toString().substring(0, 8)); - - return new UnionMoveSelectorConfig() - .withMoveSelectors(new ChangeMoveSelectorConfig(), - new SwapMoveSelectorConfig(), - new ChangeMoveSelectorConfig() - .withEntitySelectorConfig(new EntitySelectorConfig().withId(changeSelectorName)) - .withValueSelectorConfig(new ValueSelectorConfig() - .withNearbySelectionConfig(new NearbySelectionConfig() - .withOriginEntitySelectorConfig(new EntitySelectorConfig() - .withMimicSelectorRef(changeSelectorName)) - .withNearbyDistanceMeterClass( - configPolicy.getNearbyDistanceMeterClass()))), - new SwapMoveSelectorConfig() - .withEntitySelectorConfig(new EntitySelectorConfig() - .withId(swapSelectorName)) - .withSecondaryEntitySelectorConfig(new EntitySelectorConfig() - .withNearbySelectionConfig(new NearbySelectionConfig() - .withOriginEntitySelectorConfig(new EntitySelectorConfig() - .withMimicSelectorRef(swapSelectorName)) - .withNearbyDistanceMeterClass( - configPolicy.getNearbyDistanceMeterClass()))), - new TailChainSwapMoveSelectorConfig() - .withEntitySelectorConfig(new EntitySelectorConfig() - .withId(tailChainSelectorName)) - .withValueSelectorConfig(new ValueSelectorConfig() - .withNearbySelectionConfig(new NearbySelectionConfig() - .withOriginEntitySelectorConfig(new EntitySelectorConfig() - .withMimicSelectorRef(tailChainSelectorName)) - .withNearbyDistanceMeterClass( - configPolicy.getNearbyDistanceMeterClass())))); - } else { - String changeSelectorName = "changeMoveSelector-%s".formatted(UUID.randomUUID().toString().substring(0, 8)); - String swapSelectorName = "swapMoveSelector-%s".formatted(UUID.randomUUID().toString().substring(0, 8)); - return new UnionMoveSelectorConfig() - .withMoveSelectors(new ChangeMoveSelectorConfig(), - new SwapMoveSelectorConfig(), - new ChangeMoveSelectorConfig() - .withEntitySelectorConfig(new EntitySelectorConfig().withId(changeSelectorName)) - .withValueSelectorConfig(new ValueSelectorConfig() - .withNearbySelectionConfig(new NearbySelectionConfig() - .withOriginEntitySelectorConfig(new EntitySelectorConfig() - .withMimicSelectorRef(changeSelectorName)) - .withNearbyDistanceMeterClass( - configPolicy.getNearbyDistanceMeterClass()))), - new SwapMoveSelectorConfig() - .withEntitySelectorConfig(new EntitySelectorConfig() - .withId(swapSelectorName)) - .withSecondaryEntitySelectorConfig(new EntitySelectorConfig() - .withNearbySelectionConfig(new NearbySelectionConfig() - .withOriginEntitySelectorConfig(new EntitySelectorConfig() - .withMimicSelectorRef(swapSelectorName)) - .withNearbyDistanceMeterClass( - configPolicy.getNearbyDistanceMeterClass())))); - } - } + return hasChainedVariable ? determineDefaultChainedMoveSelectorConfig(configPolicy) + : determineDefaultBasicVarMoveSelectorConfig(configPolicy); } else { /* * We have a mix of basic and list variables. @@ -327,11 +221,135 @@ private UnionMoveSelectorConfig determineDefaultMoveSelectorConfig(HeuristicConf */ if (configPolicy.getNearbyDistanceMeterClass() != null) { throw new IllegalArgumentException( - "The configuration contains basic and list variables that are incompatible with using the nearbyDistanceMeterClass (%s)." + """ + The configuration contains both basic and list variables, which makes it incompatible with using a top-level nearbyDistanceMeterClass (%s). + Specify move selectors manually or remove the top-level nearbyDistanceMeterClass from your solver config.""" .formatted(configPolicy.getNearbyDistanceMeterClass())); } return new UnionMoveSelectorConfig() .withMoveSelectors(new ChangeMoveSelectorConfig(), new SwapMoveSelectorConfig()); } } + + private UnionMoveSelectorConfig determineDefaultBasicVarMoveSelectorConfig(HeuristicConfigPolicy configPolicy) { + if (configPolicy.getNearbyDistanceMeterClass() == null) { + return new UnionMoveSelectorConfig() + .withMoveSelectors(new ChangeMoveSelectorConfig(), new SwapMoveSelectorConfig()); + } else { + String changeSelectorName = addRandomSuffix("changeMoveSelector", configPolicy.getRandom()); + String swapSelectorName = addRandomSuffix("swapMoveSelector", configPolicy.getRandom()); + return new UnionMoveSelectorConfig() + .withMoveSelectors(new ChangeMoveSelectorConfig(), + new SwapMoveSelectorConfig(), + new ChangeMoveSelectorConfig() + .withEntitySelectorConfig(new EntitySelectorConfig().withId(changeSelectorName)) + .withValueSelectorConfig(new ValueSelectorConfig() + .withNearbySelectionConfig(new NearbySelectionConfig() + .withOriginEntitySelectorConfig(new EntitySelectorConfig() + .withMimicSelectorRef(changeSelectorName)) + .withNearbyDistanceMeterClass( + configPolicy.getNearbyDistanceMeterClass()))), + new SwapMoveSelectorConfig() + .withEntitySelectorConfig(new EntitySelectorConfig() + .withId(swapSelectorName)) + .withSecondaryEntitySelectorConfig(new EntitySelectorConfig() + .withNearbySelectionConfig(new NearbySelectionConfig() + .withOriginEntitySelectorConfig(new EntitySelectorConfig() + .withMimicSelectorRef(swapSelectorName)) + .withNearbyDistanceMeterClass( + configPolicy.getNearbyDistanceMeterClass())))); + } + } + + private UnionMoveSelectorConfig determineDefaultChainedMoveSelectorConfig(HeuristicConfigPolicy configPolicy) { + if (configPolicy.getNearbyDistanceMeterClass() == null) { + return new UnionMoveSelectorConfig() + .withMoveSelectors(new ChangeMoveSelectorConfig(), new SwapMoveSelectorConfig(), + new TailChainSwapMoveSelectorConfig()); + } else { + String changeSelectorName = addRandomSuffix("changeMoveSelector", configPolicy.getRandom()); + String swapSelectorName = addRandomSuffix("swapMoveSelector", configPolicy.getRandom()); + String tailChainSelectorName = addRandomSuffix("tailChainSwapMoveSelector", configPolicy.getRandom()); + return new UnionMoveSelectorConfig() + .withMoveSelectors(new ChangeMoveSelectorConfig(), + new SwapMoveSelectorConfig(), + new ChangeMoveSelectorConfig() + .withEntitySelectorConfig(new EntitySelectorConfig().withId(changeSelectorName)) + .withValueSelectorConfig(new ValueSelectorConfig() + .withNearbySelectionConfig(new NearbySelectionConfig() + .withOriginEntitySelectorConfig(new EntitySelectorConfig() + .withMimicSelectorRef(changeSelectorName)) + .withNearbyDistanceMeterClass( + configPolicy.getNearbyDistanceMeterClass()))), + new SwapMoveSelectorConfig() + .withEntitySelectorConfig(new EntitySelectorConfig() + .withId(swapSelectorName)) + .withSecondaryEntitySelectorConfig(new EntitySelectorConfig() + .withNearbySelectionConfig(new NearbySelectionConfig() + .withOriginEntitySelectorConfig(new EntitySelectorConfig() + .withMimicSelectorRef(swapSelectorName)) + .withNearbyDistanceMeterClass( + configPolicy.getNearbyDistanceMeterClass()))), + new TailChainSwapMoveSelectorConfig() + .withEntitySelectorConfig(new EntitySelectorConfig() + .withId(tailChainSelectorName)) + .withValueSelectorConfig(new ValueSelectorConfig() + .withNearbySelectionConfig(new NearbySelectionConfig() + .withOriginEntitySelectorConfig(new EntitySelectorConfig() + .withMimicSelectorRef(tailChainSelectorName)) + .withNearbyDistanceMeterClass( + configPolicy.getNearbyDistanceMeterClass())))); + } + } + + private UnionMoveSelectorConfig determineDefaultListVarMoveSelectorConfig(HeuristicConfigPolicy configPolicy) { + if (configPolicy.getNearbyDistanceMeterClass() == null) { + return new UnionMoveSelectorConfig() + .withMoveSelectors(new ListChangeMoveSelectorConfig(), new ListSwapMoveSelectorConfig(), + new KOptListMoveSelectorConfig()); + } else { + String changeSelectorName = addRandomSuffix("changeMoveSelector", configPolicy.getRandom()); + String swapSelectorName = addRandomSuffix("swapMoveSelector", configPolicy.getRandom()); + String koptSelectorName = addRandomSuffix("koptMoveSelector", configPolicy.getRandom()); + return new UnionMoveSelectorConfig() + .withMoveSelectors(new ListChangeMoveSelectorConfig(), + new ListSwapMoveSelectorConfig(), + new ListChangeMoveSelectorConfig() + .withValueSelectorConfig(new ValueSelectorConfig() + .withId(changeSelectorName)) + .withDestinationSelectorConfig(new DestinationSelectorConfig() + .withNearbySelectionConfig(new NearbySelectionConfig() + .withOriginValueSelectorConfig(new ValueSelectorConfig() + .withMimicSelectorRef(changeSelectorName)) + .withNearbyDistanceMeterClass( + configPolicy.getNearbyDistanceMeterClass()))), + new ListSwapMoveSelectorConfig() + .withValueSelectorConfig(new ValueSelectorConfig() + .withId(swapSelectorName)) + .withSecondaryValueSelectorConfig(new ValueSelectorConfig() + .withNearbySelectionConfig(new NearbySelectionConfig() + .withOriginValueSelectorConfig(new ValueSelectorConfig() + .withMimicSelectorRef(swapSelectorName)) + .withNearbyDistanceMeterClass( + configPolicy.getNearbyDistanceMeterClass()))), + new KOptListMoveSelectorConfig() + .withOriginSelectorConfig(new ValueSelectorConfig() + .withId(koptSelectorName)) + .withValueSelectorConfig(new ValueSelectorConfig() + .withNearbySelectionConfig(new NearbySelectionConfig() + .withOriginValueSelectorConfig(new ValueSelectorConfig() + .withMimicSelectorRef(koptSelectorName)) + .withNearbyDistanceMeterClass( + configPolicy.getNearbyDistanceMeterClass())))); + } + } + + private String addRandomSuffix(String name, Random random) { + StringBuilder value = new StringBuilder(name); + value.append("-"); + random.ints(97, 122) // ['a', 'z'] + .limit(4) // 4 letters + .forEach(value::appendCodePoint); + return value.toString(); + } } diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolverFactory.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolverFactory.java index 08947f8b06..afe00b172f 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolverFactory.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolverFactory.java @@ -108,12 +108,15 @@ public Solver buildSolver(SolverConfigOverride configOverr var moveThreadCount = resolveMoveThreadCount(true); var bestSolutionRecaller = BestSolutionRecallerFactory.create(). buildBestSolutionRecaller(environmentMode); + var randomFactory = buildRandomFactory(environmentMode); + var configPolicy = new HeuristicConfigPolicy.Builder<>( environmentMode, moveThreadCount, solverConfig.getMoveThreadBufferSize(), solverConfig.getThreadFactoryClass(), solverConfig.getNearbyDistanceMeterClass(), + randomFactory.createRandom(), scoreDirectorFactory.getInitializingScoreTrend(), solutionDescriptor, ClassInstanceCache.create()).build(); @@ -121,7 +124,6 @@ public Solver buildSolver(SolverConfigOverride configOverr var termination = buildTerminationConfig(basicPlumbingTermination, configPolicy, configOverride); var phaseList = buildPhaseList(configPolicy, bestSolutionRecaller, termination); - var randomFactory = buildRandomFactory(environmentMode); return new DefaultSolver<>(environmentMode, randomFactory, bestSolutionRecaller, basicPlumbingTermination, termination, phaseList, solverScope, moveThreadCount == null ? SolverConfig.MOVE_THREAD_COUNT_NONE : Integer.toString(moveThreadCount)); diff --git a/core/core-impl/src/test/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicyTestUtils.java b/core/core-impl/src/test/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicyTestUtils.java index 4179bf36fd..f28fd0cac6 100644 --- a/core/core-impl/src/test/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicyTestUtils.java +++ b/core/core-impl/src/test/java/ai/timefold/solver/core/impl/heuristic/HeuristicConfigPolicyTestUtils.java @@ -1,5 +1,7 @@ package ai.timefold.solver.core.impl.heuristic; +import java.util.Random; + import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; import ai.timefold.solver.core.impl.solver.ClassInstanceCache; @@ -13,7 +15,7 @@ public static HeuristicConfigPolicy buildHeuristicConfigPolicy public static HeuristicConfigPolicy buildHeuristicConfigPolicy(SolutionDescriptor solutionDescriptor) { - return new HeuristicConfigPolicy.Builder<>(EnvironmentMode.REPRODUCIBLE, null, null, null, null, null, + return new HeuristicConfigPolicy.Builder<>(EnvironmentMode.REPRODUCIBLE, null, null, null, null, new Random(), null, solutionDescriptor, ClassInstanceCache.create()) .build(); }