feat: improve the logic for random iterators of entity range selectors

Author: zepfred

core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/value/decorator/FilteringValueRangeSelector.java

@@ -129,20 +129,31 @@ public Iterator<Object> iterator(Object entity) {
     @Override
     public Iterator<Object> iterator() {
         if (randomSelection) {
-            return new RandomFilteringValueRangeIterator(replayingValueSelector.iterator(), listVariableStateSupply,
-                    reachableValues, workingRandom, (int) getSize(), checkSourceAndDestination, true);
+            // If the nonReplayingValueSelector does not have any additional configuration,
+            // we can bypass it and only use reachable values,
+            // which helps optimize the number of evaluations.
+            // However, if the nonReplayingValueSelector includes custom configurations,
+            // such as filtering,
+            // we will first evaluate its values and then filter out those that are not reachable.
+            if (nonReplayingValueSelector instanceof IterableFromEntityPropertyValueSelector<Solution_>) {
+                return new OptimizedRandomFilteringValueRangeIterator(replayingValueSelector.iterator(),
+                        listVariableStateSupply,
+                        reachableValues, workingRandom, (int) getSize(), checkSourceAndDestination);
+            } else {
+                return new RandomFilteringValueRangeIterator(replayingValueSelector.iterator(),
+                        nonReplayingValueSelector.iterator(), listVariableStateSupply, reachableValues, (int) getSize(),
+                        checkSourceAndDestination);
+            }
         } else {
             return new OriginalFilteringValueRangeIterator(replayingValueSelector.iterator(),
-                    nonReplayingValueSelector.iterator(), listVariableStateSupply, reachableValues, checkSourceAndDestination,
-                    false);
+                    nonReplayingValueSelector.iterator(), listVariableStateSupply, reachableValues, checkSourceAndDestination);
         }
     }
 
     @Override
     public Iterator<Object> endingIterator(Object entity) {
         return new OriginalFilteringValueRangeIterator(replayingValueSelector.iterator(),
-                nonReplayingValueSelector.iterator(), listVariableStateSupply, reachableValues, checkSourceAndDestination,
-                false);
+                nonReplayingValueSelector.iterator(), listVariableStateSupply, reachableValues, checkSourceAndDestination);
     }
 
     @Override
@@ -258,7 +269,7 @@ boolean isValueOrEntityReachable(Object destinationValue) {
     private class OriginalFilteringValueRangeIterator extends AbstractFilteringValueRangeIterator {
         // The value iterator that only replays the current selected value
         private final Iterator<Object> replayingValueIterator;
-        // The value iterator returns all possible values based on its settings.
+        // The value iterator returns all possible values based on the outer selector settings.
         // However,
         // it may include invalid values that need to be filtered out.
         // This iterator must be used to ensure that all positions are included in the CH phase.
@@ -267,8 +278,8 @@ private class OriginalFilteringValueRangeIterator extends AbstractFilteringValue
 
         private OriginalFilteringValueRangeIterator(Iterator<Object> replayingValueIterator, Iterator<Object> valueIterator,
                 ListVariableStateSupply<Solution_> listVariableStateSupply, ReachableValues reachableValues,
-                boolean checkSourceAndDestination, boolean useValueList) {
-            super(listVariableStateSupply, reachableValues, checkSourceAndDestination, useValueList);
+                boolean checkSourceAndDestination) {
+            super(listVariableStateSupply, reachableValues, checkSourceAndDestination, false);
             this.replayingValueIterator = replayingValueIterator;
             this.valueIterator = valueIterator;
         }
@@ -307,15 +318,71 @@ protected Object createUpcomingSelection() {
     }
 
     private class RandomFilteringValueRangeIterator extends AbstractFilteringValueRangeIterator {
+        // The value iterator that only replays the current selected value
+        private final Iterator<Object> replayingValueIterator;
+        // The value iterator returns all possible values based on the outer selector settings.
+        private final Iterator<Object> valueIterator;
+        private final int maxBailoutSize;
+
+        private RandomFilteringValueRangeIterator(Iterator<Object> replayingValueIterator, Iterator<Object> valueIterator,
+                ListVariableStateSupply<Solution_> listVariableStateSupply, ReachableValues reachableValues,
+                int maxBailoutSize, boolean checkSourceAndDestination) {
+            super(listVariableStateSupply, reachableValues, checkSourceAndDestination, false);
+            this.replayingValueIterator = replayingValueIterator;
+            this.valueIterator = valueIterator;
+            this.maxBailoutSize = maxBailoutSize;
+        }
+
+        private void initialize() {
+            if (initialized) {
+                return;
+            }
+            if (replayingValueIterator.hasNext()) {
+                var upcomingValue = replayingValueIterator.next();
+                if (!valueIterator.hasNext()) {
+                    noData();
+                } else {
+                    loadValues(Objects.requireNonNull(upcomingValue));
+                }
+            } else {
+                noData();
+            }
+        }
+
+        @Override
+        protected Object createUpcomingSelection() {
+            initialize();
+            if (!hasData) {
+                return noUpcomingSelection();
+            }
+            Object next;
+            var bailoutSize = maxBailoutSize;
+            do {
+                if (bailoutSize <= 0 || !valueIterator.hasNext()) {
+                    return noUpcomingSelection();
+                }
+                bailoutSize--;
+                next = valueIterator.next();
+            } while (!isValueOrEntityReachable(next));
+            return next;
+        }
+    }
+
+    /**
+     * The optimized iterator only traverses reachable values from the current selection.
+     * Unlike {@link RandomFilteringValueRangeIterator},
+     * it does not use an outer iterator to filter out non-reachable values.
+     */
+    private class OptimizedRandomFilteringValueRangeIterator extends AbstractFilteringValueRangeIterator {
 
         private final Iterator<Object> replayingValueIterator;
         private final Random workingRandom;
         private final int maxBailoutSize;
 
-        private RandomFilteringValueRangeIterator(Iterator<Object> replayingValueIterator,
+        private OptimizedRandomFilteringValueRangeIterator(Iterator<Object> replayingValueIterator,
                 ListVariableStateSupply<Solution_> listVariableStateSupply, ReachableValues reachableValues,
-                Random workingRandom, int maxBailoutSize, boolean checkSourceAndDestination, boolean useValueList) {
-            super(listVariableStateSupply, reachableValues, checkSourceAndDestination, useValueList);
+                Random workingRandom, int maxBailoutSize, boolean checkSourceAndDestination) {
+            super(listVariableStateSupply, reachableValues, checkSourceAndDestination, true);
             this.replayingValueIterator = replayingValueIterator;
             this.workingRandom = workingRandom;
             this.maxBailoutSize = maxBailoutSize;

core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java

@@ -365,7 +365,8 @@ void solveWithEntityValueRangeBasicVariable() {
     @Test
     void solveWithEntityValueRangeListVariable() {
         var solverConfig = PlannerTestUtils
-                .buildSolverConfig(TestdataListEntityProvidingSolution.class, TestdataListEntityProvidingEntity.class)
+                .buildSolverConfig(TestdataListEntityProvidingSolution.class, TestdataListEntityProvidingEntity.class,
+                        TestdataListEntityProvidingValue.class)
                 .withEasyScoreCalculatorClass(TestdataListEntityProvidingScoreCalculator.class)
                 .withPhases(new ConstructionHeuristicPhaseConfig());
 
@@ -381,8 +382,10 @@ void solveWithEntityValueRangeListVariable() {
         var bestSolution = PlannerTestUtils.solve(solverConfig, solution, true);
         assertThat(bestSolution).isNotNull();
         // Only one entity should provide the value list and assign the values.
-        assertThat(bestSolution.getEntityList().get(0).getValueList()).hasSameElementsAs(List.of(value1, value2));
-        assertThat(bestSolution.getEntityList().get(1).getValueList()).hasSameElementsAs(List.of(value3));
+        assertThat(bestSolution.getEntityList().get(0).getValueList().stream().map(TestdataListEntityProvidingValue::getCode))
+                .hasSameElementsAs(List.of("v1", "v2"));
+        assertThat(bestSolution.getEntityList().get(1).getValueList().stream().map(TestdataListEntityProvidingValue::getCode))
+                .hasSameElementsAs(List.of("v3"));
     }
 
     @Test

core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/list/ElementDestinationSelectorTest.java

@@ -9,6 +9,7 @@
 import static ai.timefold.solver.core.testdomain.list.TestdataListUtils.getPinnedAllowsUnassignedvaluesListVariableDescriptor;
 import static ai.timefold.solver.core.testdomain.list.TestdataListUtils.getPinnedListVariableDescriptor;
 import static ai.timefold.solver.core.testdomain.list.TestdataListUtils.mockEntitySelector;
+import static ai.timefold.solver.core.testdomain.list.TestdataListUtils.mockIterableFromEntityPropertyValueSelector;
 import static ai.timefold.solver.core.testdomain.list.TestdataListUtils.mockIterableValueSelector;
 import static ai.timefold.solver.core.testutil.PlannerAssert.assertAllCodesOfIterableSelector;
 import static ai.timefold.solver.core.testutil.PlannerAssert.assertAllCodesOfIterator;
@@ -175,11 +176,11 @@ void randomWithEntityValueRange() {
         // 1 - pick random value in ElementPositionRandomIterator and return the first unpinned position
         // 1 - remaining call
         var valueSelector = mockIterableValueSelector(getEntityRangeListVariableDescriptor(scoreDirector), v3);
+        var filteringValueRangeSelector = mockIterableFromEntityPropertyValueSelector(valueSelector, true);
         var replayinValueSelector = mockIterableValueSelector(getEntityRangeListVariableDescriptor(scoreDirector), v3);
         checkEntityValueRange(new FilteringEntityValueRangeSelector<>(mockEntitySelector(a, b, c), valueSelector, true),
-                new FilteringValueRangeSelector<>(valueSelector, replayinValueSelector, true, false), scoreDirector,
-                new TestRandom(1, 1, 1),
-                "C[0]");
+                new FilteringValueRangeSelector<>(filteringValueRangeSelector, replayinValueSelector, true, false),
+                scoreDirector, new TestRandom(1, 1, 1), "C[0]");
 
         // select A for V1 and random pos A[2]
         // Random values
@@ -188,38 +189,41 @@ void randomWithEntityValueRange() {
         // 0 - pick random position, only v2 is reachable
         // 0 - remaining call
         valueSelector = mockIterableValueSelector(getEntityRangeListVariableDescriptor(scoreDirector), v1);
+        filteringValueRangeSelector = mockIterableFromEntityPropertyValueSelector(valueSelector, true);
         replayinValueSelector = mockIterableValueSelector(getEntityRangeListVariableDescriptor(scoreDirector), v1);
         // Cause the value iterator return no value at the second call
         doReturn(List.of(v1).iterator(), Collections.emptyIterator()).when(valueSelector).iterator();
         checkEntityValueRange(new FilteringEntityValueRangeSelector<>(mockEntitySelector(a, b, c), valueSelector, true),
-                new FilteringValueRangeSelector<>(valueSelector, replayinValueSelector, true, false), scoreDirector,
-                new TestRandom(0, 3, 0, 0), "A[2]");
+                new FilteringValueRangeSelector<>(filteringValueRangeSelector, replayinValueSelector, true, false),
+                scoreDirector, new TestRandom(0, 3, 0, 0), "A[2]");
 
         // select B for V1 and random pos B[1]
         // 1 - pick entity B in RandomFilteringValueRangeIterator
         // 3 - pick a random value in ElementPositionRandomIterator and force generating a random position
         // 1 - pick random position, v1 and v3 are reachable
         // 0 - remaining call
         valueSelector = mockIterableValueSelector(getEntityRangeListVariableDescriptor(scoreDirector), v2, v2, v2, v2, v2); // simulate five positions
+        filteringValueRangeSelector = mockIterableFromEntityPropertyValueSelector(valueSelector, true);
         replayinValueSelector = mockIterableValueSelector(getEntityRangeListVariableDescriptor(scoreDirector), v2);
         // Cause the value iterator return no value at the second call
         doReturn(List.of(v2).iterator(), Collections.emptyIterator()).when(valueSelector).iterator();
         checkEntityValueRange(new FilteringEntityValueRangeSelector<>(mockEntitySelector(a, b, c), valueSelector, true),
-                new FilteringValueRangeSelector<>(valueSelector, replayinValueSelector, true, false), scoreDirector,
-                new TestRandom(1, 3, 1, 0), "B[1]");
+                new FilteringValueRangeSelector<>(filteringValueRangeSelector, replayinValueSelector, true, false),
+                scoreDirector, new TestRandom(1, 3, 1, 0), "B[1]");
 
         // select C for V5 and first unpinned pos C[1]
         // 0 - pick entity C in RandomFilteringValueRangeIterator
         // 3 - pick random value in ElementPositionRandomIterator and force generating a random position
         // 1 - pick random position, v3 and v4 are reachable
         // 0 - remaining call
         valueSelector = mockIterableValueSelector(getEntityRangeListVariableDescriptor(scoreDirector), v5, v5, v5, v5, v5); // simulate five positions
+        filteringValueRangeSelector = mockIterableFromEntityPropertyValueSelector(valueSelector, true);
         replayinValueSelector = mockIterableValueSelector(getEntityRangeListVariableDescriptor(scoreDirector), v5);
         // Cause the value iterator return no value at the second call
         doReturn(List.of(v5).iterator(), Collections.emptyIterator()).when(valueSelector).iterator();
         checkEntityValueRange(new FilteringEntityValueRangeSelector<>(mockEntitySelector(a, b, c), valueSelector, true),
-                new FilteringValueRangeSelector<>(valueSelector, replayinValueSelector, true, false), scoreDirector,
-                new TestRandom(0, 3, 1, 0), "C[1]");
+                new FilteringValueRangeSelector<>(filteringValueRangeSelector, replayinValueSelector, true, false),
+                scoreDirector, new TestRandom(0, 3, 1, 0), "C[1]");
     }
 
     private void checkEntityValueRange(FilteringEntityValueRangeSelector<TestdataListEntityProvidingSolution> entitySelector,

core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/ListChangeMoveSelectorTest.java

@@ -25,6 +25,8 @@
 import java.util.List;
 import java.util.Random;
 
+import ai.timefold.solver.core.api.score.director.ScoreDirector;
+import ai.timefold.solver.core.impl.heuristic.selector.common.decorator.SelectionFilter;
 import ai.timefold.solver.core.preview.api.domain.metamodel.ElementPosition;
 import ai.timefold.solver.core.testdomain.TestdataValue;
 import ai.timefold.solver.core.testdomain.list.TestdataListEntity;
@@ -415,6 +417,40 @@ void randomWithEntityValueRange() {
                 "3 {B[0]->B[0]}");
     }
 
+    @Test
+    void randomWithEntityValueRangeAndFiltering() {
+        var v1 = new TestdataListEntityProvidingValue("1");
+        var v2 = new TestdataListEntityProvidingValue("2");
+        var v3 = new TestdataListEntityProvidingValue("3");
+        var a = new TestdataListEntityProvidingEntity("A", List.of(v1, v2), List.of(v2, v1));
+        var b = new TestdataListEntityProvidingEntity("B", List.of(v2, v3), List.of(v3));
+        var solution = new TestdataListEntityProvidingSolution();
+        solution.setEntityList(List.of(a, b));
+
+        var scoreDirector = mockScoreDirector(TestdataListEntityProvidingSolution.buildSolutionDescriptor());
+        scoreDirector.setWorkingSolution(solution);
+
+        var mimicRecordingValueSelector = getMimicRecordingIterableValueSelector(
+                getEntityRangeListVariableDescriptor(scoreDirector).getValueRangeDescriptor(), true);
+        var solutionDescriptor = scoreDirector.getSolutionDescriptor();
+        var entityDescriptor = solutionDescriptor.findEntityDescriptor(TestdataListEntityProvidingEntity.class);
+        var destinationSelector = getEntityValueRangeDestinationSelector(mimicRecordingValueSelector, solutionDescriptor,
+                entityDescriptor, IgnoreBValueSelectionFilter.class, true);
+        var moveSelector = new ListChangeMoveSelector<>(mimicRecordingValueSelector, destinationSelector, true);
+
+        var solverScope = solvingStarted(moveSelector, scoreDirector, new Random(0));
+        phaseStarted(solverScope, moveSelector);
+
+        // IgnoreBValueSelectionFilter is applied to the value selector used by the destination selector,
+        // and that causes the B destination to become an invalid destination
+        assertCodesOfNeverEndingMoveSelector(moveSelector,
+                "1 {A[1]->A[1]}",
+                "3 {B[0]->A[1]}",
+                "1 {A[1]->A[1]}",
+                "1 {A[1]->A[1]}",
+                "1 {A[1]->A[0]}");
+    }
+
     @Test
     void randomWithPinning() {
         var v1 = new TestdataPinnedWithIndexListValue("1");
@@ -576,4 +612,18 @@ void randomAllowsUnassignedValuesWithEntityValueRange() {
                 "3 {B[0]->B[0]}",
                 "3 {B[0]->B[0]}");
     }
+
+    public static class IgnoreBValueSelectionFilter
+            implements SelectionFilter<TestdataListEntityProvidingSolution, TestdataListEntityProvidingValue> {
+
+        public IgnoreBValueSelectionFilter() {
+            // Required for solver initialization
+        }
+
+        @Override
+        public boolean accept(ScoreDirector<TestdataListEntityProvidingSolution> scoreDirector,
+                TestdataListEntityProvidingValue selection) {
+            return !selection.getEntity().getCode().equals("B");
+        }
+    }
 }