feat: improve unassign move generation

core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/common/iterator/UpcomingSelectionIterator.java

@@ -22,14 +22,24 @@ public abstract class UpcomingSelectionIterator<S> extends SelectionIterator<S>
 
     protected boolean upcomingCreated = false;
     protected boolean hasUpcomingSelection = true;
+    private boolean recheckUpcomingSelection = false;
     protected S upcomingSelection;
 
-    @Override
-    public boolean hasNext() {
+    private void readAndRecheckUpcomingValue() {
         if (!upcomingCreated) {
             upcomingSelection = createUpcomingSelection();
             upcomingCreated = true;
+            if (recheckUpcomingSelection) {
+                // We ensure that the variable remains consistent if "discardUpcomingSelection" was called previously.
+                hasUpcomingSelection = upcomingSelection != null;
+                recheckUpcomingSelection = false;
+            }
         }
+    }
+
+    @Override
+    public boolean hasNext() {
+        readAndRecheckUpcomingValue();
         return hasUpcomingSelection;
     }
 
@@ -38,9 +48,7 @@ public S next() {
         if (!hasUpcomingSelection) {
             throw new NoSuchElementException();
         }
-        if (!upcomingCreated) {
-            upcomingSelection = createUpcomingSelection();
-        }
+        readAndRecheckUpcomingValue();
         upcomingCreated = false;
         return upcomingSelection;
     }
@@ -54,6 +62,7 @@ protected S noUpcomingSelection() {
 
     public void discardUpcomingSelection() {
         upcomingCreated = false;
+        recheckUpcomingSelection = true;
     }
 
     @Override

core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/list/ElementDestinationSelector.java

@@ -128,7 +128,7 @@ public Iterator<ElementPosition> iterator() {
             var totalSize = Math.addExact(entitySelector.getSize(), totalValueSize);
             return new ElementPositionRandomIterator<>(listVariableStateSupply, entitySelector,
                     replayingValueSelector != null ? replayingValueSelector.iterator() : null, valueSelector, workingRandom,
-                    totalSize, allowsUnassignedValues);
+                    totalSize, allowsUnassignedValues, allowsUnassignedValues && totalValueSize > 0);
         } else {
             if (entitySelector.getSize() == 0) {
                 return Collections.emptyIterator();

core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/list/ElementPositionRandomIterator.java

@@ -1,6 +1,7 @@
 package ai.timefold.solver.core.impl.heuristic.selector.list;
 
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 import java.util.random.RandomGenerator;
 
 import ai.timefold.solver.core.impl.domain.variable.ListVariableStateSupply;
@@ -23,14 +24,15 @@ final class ElementPositionRandomIterator<Solution_> implements Iterator<Element
     private final RandomGenerator workingRandom;
     private final long totalSize;
     private final boolean allowsUnassignedValues;
+    private final boolean maybeMovableValues;
     private Iterator<Object> valueIterator;
     private Object selectedValue;
     private boolean hasNextValue = false;
 
     public ElementPositionRandomIterator(ListVariableStateSupply<Solution_, Object, Object> listVariableStateSupply,
             EntitySelector<Solution_> entitySelector, Iterator<Object> replayingValueIterator,
             IterableValueSelector<Solution_> valueSelector, RandomGenerator workingRandom, long totalSize,
-            boolean allowsUnassignedValues) {
+            boolean allowsUnassignedValues, boolean maybeMovableValues) {
         this.listVariableStateSupply = listVariableStateSupply;
         this.listVariableDescriptor = listVariableStateSupply.getSourceVariableDescriptor();
         this.entitySelector = entitySelector;
@@ -44,6 +46,7 @@ public ElementPositionRandomIterator(ListVariableStateSupply<Solution_, Object,
                     .formatted(totalSize));
         }
         this.allowsUnassignedValues = allowsUnassignedValues;
+        this.maybeMovableValues = maybeMovableValues;
         this.valueIterator = null;
     }
 
@@ -72,9 +75,16 @@ private boolean hasNextValue() {
                 // and the entity iterator must discard the previous entity
                 tryUpdateEntityIterator();
             }
-            return entityIterator.hasNext();
+            // There will be a valid destination if the entity iterator has a next element 
+            // or if there is at least one non-pinned assigned value,
+            // which would result in an unassigning move.
+            return entityIterator.hasNext() || maybeMovableValues;
         }
-        return selectedValue != null && entityIterator.hasNext();
+        // There will be a valid destination if the entity iterator has a next element
+        // or if there is at least one non-pinned assigned value,
+        // which would result in an unassigning move.
+        return selectedValue != null
+                && (entityIterator.hasNext() || maybeMovableValues);
     }
 
     @Override
@@ -88,6 +98,9 @@ public boolean hasNext() {
 
     @Override
     public ElementPosition next() {
+        if (!hasNextValue) {
+            throw new NoSuchElementException();
+        }
         this.hasNextValue = false;
         // This code operates under the assumption that the entity selector already filtered out all immovable entities.
         // At this point, entities are only partially pinned, or not pinned at all.
@@ -96,7 +109,7 @@ public ElementPosition next() {
         // to account for the unassigned destination, which is an extra element.
         var entityBoundary = allowsUnassignedValues ? entitySize + 1 : entitySize;
         var random = RandomUtils.nextLong(workingRandom, allowsUnassignedValues ? totalSize + 1 : totalSize);
-        if (allowsUnassignedValues && random == 0) {
+        if (allowsUnassignedValues && (random == 0 || !entityIterator.hasNext())) {
             // We have already excluded all unassigned elements,
             // the only way to get an unassigned destination is to explicitly add it.
             return ElementPosition.unassigned();

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

@@ -12,6 +12,7 @@
 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.testdomain.list.TestdataListUtils.mockUpcomingSelectionIterator;
 import static ai.timefold.solver.core.testutil.PlannerAssert.assertAllCodesOfIterableSelector;
 import static ai.timefold.solver.core.testutil.PlannerAssert.assertAllCodesOfIterator;
 import static ai.timefold.solver.core.testutil.PlannerAssert.assertCodesOfNeverEndingIterableSelector;
@@ -21,25 +22,33 @@
 import static ai.timefold.solver.core.testutil.PlannerAssert.verifyPhaseLifecycle;
 import static ai.timefold.solver.core.testutil.PlannerTestUtils.mockScoreDirector;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
 import java.util.Collections;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.Random;
 
 import ai.timefold.solver.core.api.solver.SolutionManager;
 import ai.timefold.solver.core.config.heuristic.selector.common.SelectionCacheType;
+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.entity.FromSolutionEntitySelector;
 import ai.timefold.solver.core.impl.heuristic.selector.entity.decorator.FilteringEntityByValueSelector;
+import ai.timefold.solver.core.impl.heuristic.selector.entity.decorator.FilteringEntitySelector;
 import ai.timefold.solver.core.impl.heuristic.selector.value.IterableValueSelector;
 import ai.timefold.solver.core.impl.heuristic.selector.value.decorator.FilteringValueRangeSelector;
+import ai.timefold.solver.core.impl.heuristic.selector.value.mimic.ManualValueMimicRecorder;
+import ai.timefold.solver.core.impl.heuristic.selector.value.mimic.MimicReplayingValueSelector;
 import ai.timefold.solver.core.impl.localsearch.scope.LocalSearchPhaseScope;
 import ai.timefold.solver.core.impl.score.director.InnerScoreDirector;
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
+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;
 import ai.timefold.solver.core.testdomain.list.TestdataListSolution;
 import ai.timefold.solver.core.testdomain.list.TestdataListUtils;
@@ -56,6 +65,8 @@
 import ai.timefold.solver.core.testdomain.list.valuerange.TestdataListEntityProvidingEntity;
 import ai.timefold.solver.core.testdomain.list.valuerange.TestdataListEntityProvidingSolution;
 import ai.timefold.solver.core.testdomain.list.valuerange.TestdataListEntityProvidingValue;
+import ai.timefold.solver.core.testdomain.list.valuerange.unassignedvar.pinned.TestdataListUnassignedPinnedEntityProvidingEntity;
+import ai.timefold.solver.core.testdomain.list.valuerange.unassignedvar.pinned.TestdataListUnassignedPinnedEntityProvidingSolution;
 import ai.timefold.solver.core.testutil.TestRandom;
 
 import org.junit.jupiter.api.Test;
@@ -472,6 +483,65 @@ void randomFullyPinned() {
                 "B[0]");
     }
 
+    @Test
+    void refreshReachableEntities() {
+        var v1 = new TestdataValue("1");
+        var v2 = new TestdataValue("2");
+        var a = new TestdataListUnassignedPinnedEntityProvidingEntity("A", List.of(v1)); // Pinned
+        var b = new TestdataListUnassignedPinnedEntityProvidingEntity("B", List.of(v2)); // Not pinned
+        // a is pinned
+        a.setPinned(true);
+        a.setValueList(List.of(v1));
+        b.setValueList(List.of(v2));
+        var solution = new TestdataListUnassignedPinnedEntityProvidingSolution();
+        solution.setEntityList(List.of(a, b));
+        SolutionManager.updateShadowVariables(solution);
+
+        var scoreDirector = mockScoreDirector(TestdataListUnassignedPinnedEntityProvidingSolution.buildSolutionDescriptor());
+        scoreDirector.setWorkingSolution(solution);
+
+        // Value selector
+        var listVariableDescriptor = TestdataListUnassignedPinnedEntityProvidingEntity.buildVariableDescriptorForValueList();
+        var iterableValueSelector = mockIterableValueSelector(listVariableDescriptor, v1, v2);
+        var mimicRecorder = new ManualValueMimicRecorder<>(iterableValueSelector);
+        var replayingValueSelector = new MimicReplayingValueSelector<>(mimicRecorder);
+        // Entity selector with non-pinned entity filtered by value
+        var entityDescriptor = TestdataListUnassignedPinnedEntityProvidingEntity.buildEntityDescriptor();
+        var entitySelector = new FromSolutionEntitySelector<>(entityDescriptor, SelectionCacheType.PHASE, true);
+        var filteringEntity = new FilteringEntityByValueSelector<>(entitySelector, replayingValueSelector, true, false);
+        var pinningFilterFunction = entityDescriptor.getEffectiveMovableEntityFilter();
+        var nonPinnedEntitySelector = FilteringEntitySelector.of(filteringEntity, SelectionFilter
+                .compose((director, selection) -> pinningFilterFunction.test(director.getWorkingSolution(), selection)));
+        // Destination selector with non-pinned entity selector filtered by value
+        var selector =
+                new ElementDestinationSelector<>(nonPinnedEntitySelector, replayingValueSelector, iterableValueSelector, true,
+                        false);
+
+        // First, we select v1, which is pinned, and the entity iterator does not return a feasible destination.
+        // However, the value selector has another assigned value,
+        // which makes maybeMovableValues in ElementDestinationSelector to be set to true
+        // We always return 0 to meet the bailout size,
+        // and then return 1 to ensure the entity is selected by destination iterator
+        var random = new TestRandom(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1);
+        var solverScope = solvingStarted(selector, scoreDirector, random);
+        phaseStarted(selector, solverScope);
+        var iterator = selector.iterator();
+        mimicRecorder.setRecordedValue(v1);
+        assertThat(iterator.hasNext()).isTrue();
+        // The expected position is unassigned as v1 has no feasible destination
+        assertThat(iterator.next()).isSameAs(ElementPosition.unassigned());
+        // Next, we select v2, which is not pinned, and the entity iterator returns a feasible destination.
+        // This will cause the iterator to call tryUpdateEntityIterator, and reload the entity list
+        // b is the only reachable non-pinned entity for v2
+        mimicRecorder.setRecordedValue(v2);
+        assertThat(iterator.hasNext()).isTrue();
+        var position = iterator.next().ensureAssigned();
+        var entity = position.entity();
+        var index = position.index();
+        assertThat(entity).isSameAs(b);
+        assertThat(index).isSameAs(0);
+    }
+
     @Test
     void emptyIfThereAreNoEntities() {
         var v1 = new TestdataListValue("1");
@@ -614,4 +684,41 @@ void discardOldValues() {
         // the entity iterator must discard the previous entity during the hasNext() calls
         verify(entityIterator, times(1)).discardUpcomingSelection();
     }
+
+    @Test
+    void discardOldValuesAndResetState() {
+        var v1 = new TestdataListEntityProvidingValue("V1");
+        var v2 = new TestdataListEntityProvidingValue("V2");
+        var a = new TestdataListEntityProvidingEntity("A", List.of(), List.of());
+        var solution = new TestdataListEntityProvidingSolution();
+        solution.setEntityList(List.of(a));
+
+        var scoreDirector = mockScoreDirector(TestdataListEntityProvidingSolution.buildSolutionDescriptor());
+        scoreDirector.setWorkingSolution(solution);
+
+        var entitySelector = mockEntitySelector(a);
+        var entityIterator = mockUpcomingSelectionIterator(a, null, a);
+        doReturn(entityIterator).when(entitySelector).iterator();
+        var valueSelector = mockIterableValueSelector(getEntityRangeListVariableDescriptor(scoreDirector), v1);
+        IterableValueSelector<TestdataListEntityProvidingSolution> replayingValueSelector =
+                mockReplayingValueSelector(getEntityRangeListVariableDescriptor(scoreDirector), v1, v1, v2);
+
+        var selector = new ElementDestinationSelector<>(entitySelector, replayingValueSelector, valueSelector, true, false);
+        // Value 0 makes the iterator to always request an entity from the related iterator
+        var random = new TestRandom(0, 0);
+        solvingStarted(selector, scoreDirector, random);
+        var iterator = selector.iterator();
+        // entityIterator returns a
+        assertThat(iterator.hasNext()).isTrue();
+        assertThat(iterator.next()).isNotNull();
+        // entityIterator gets null and call noUpcomingSelection
+        assertThat(iterator.hasNext()).isFalse();
+        assertThatCode(iterator::next).isInstanceOf(NoSuchElementException.class);
+        // replayingValueSelector returns v2, discardUpcomingSelection is called, and entityIterator returns a again
+        assertThat(iterator.hasNext()).isTrue();
+        assertThat(iterator.next()).isNotNull();
+        // iterator exhausted again
+        assertThat(iterator.hasNext()).isFalse();
+        assertThatCode(iterator::next).isInstanceOf(NoSuchElementException.class);
+    }
 }

core/src/test/java/ai/timefold/solver/core/testdomain/list/TestdataListUtils.java

@@ -9,6 +9,7 @@
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Objects;
 
 import ai.timefold.solver.core.config.heuristic.selector.common.SelectionCacheType;
 import ai.timefold.solver.core.config.heuristic.selector.entity.EntitySelectorConfig;
@@ -21,6 +22,7 @@
 import ai.timefold.solver.core.impl.heuristic.HeuristicConfigPolicy;
 import ai.timefold.solver.core.impl.heuristic.selector.SelectorTestUtils;
 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.entity.EntitySelector;
 import ai.timefold.solver.core.impl.heuristic.selector.list.DestinationSelector;
 import ai.timefold.solver.core.impl.heuristic.selector.list.DestinationSelectorFactory;
@@ -215,6 +217,10 @@ public static DestinationSelector<TestdataPinnedWithIndexListSolution> mockPinne
         return mockNeverEndingDestinationSelector(locationsInList.length, locationsInList);
     }
 
+    public static <Selection_> MockedUpcomingSelectionIterator<Selection_> mockUpcomingSelectionIterator(Selection_... values) {
+        return new MockedUpcomingSelectionIterator<>(values);
+    }
+
     public static <Solution_> DestinationSelector<Solution_> mockNeverEndingDestinationSelector(long size,
             ElementPosition... locationsInList) {
         var destinationSelector = mock(DestinationSelector.class);
@@ -372,6 +378,25 @@ public static <S> DestinationSelector<S> getEntityValueRangeDestinationSelector(
                 .buildDestinationSelector(configPolicy, SelectionCacheType.JUST_IN_TIME, randomSelection, "any", false);
     }
 
+    public static class MockedUpcomingSelectionIterator<Selection_> extends UpcomingSelectionIterator<Selection_> {
+
+        private final Selection_[] values;
+        private int index = 0;
+
+        public MockedUpcomingSelectionIterator(Selection_[] values) {
+            this.values = Objects.requireNonNull(values);
+        }
+
+        @Override
+        protected Selection_ createUpcomingSelection() {
+            if (index >= values.length || values[index] == null) {
+                index++;
+                return noUpcomingSelection();
+            }
+            return values[index++];
+        }
+    }
+
     private static <T> Iterator<T> cyclicIterator(List<T> elements) {
         if (elements.isEmpty()) {
             return Collections.emptyIterator();