fix: filter trivial 2-opt moves and make KOptListMove public

core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/KOptListMove.java

@@ -16,7 +16,7 @@
 /**
  * @param <Solution_> the solution type, the class with the {@link PlanningSolution} annotation
  */
-final class KOptListMove<Solution_> extends AbstractMove<Solution_> {
+public final class KOptListMove<Solution_> extends AbstractMove<Solution_> {
 
     private final ListVariableDescriptor<Solution_> listVariableDescriptor;
     private final KOptDescriptor<?> descriptor;
@@ -27,7 +27,7 @@ final class KOptListMove<Solution_> extends AbstractMove<Solution_> {
     private final int[] newEndIndices;
     private final Object[] originalEntities;
 
-    public KOptListMove(ListVariableDescriptor<Solution_> listVariableDescriptor,
+    KOptListMove(ListVariableDescriptor<Solution_> listVariableDescriptor,
             SingletonInverseVariableSupply inverseVariableSupply,
             KOptDescriptor<?> descriptor,
             List<FlipSublistAction> equivalent2Opts,
@@ -59,7 +59,7 @@ public KOptListMove(ListVariableDescriptor<Solution_> listVariableDescriptor,
         }
     }
 
-    public KOptListMove(ListVariableDescriptor<Solution_> listVariableDescriptor,
+    KOptListMove(ListVariableDescriptor<Solution_> listVariableDescriptor,
             KOptDescriptor<?> descriptor,
             List<FlipSublistAction> equivalent2Opts,
             MultipleDelegateList<?> combinedList,

core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMove.java

@@ -19,24 +19,29 @@
  * [A, B, C, D, E, F, G, H]. The edge (B, E) became (B, C), and the edge (C, F) became (E, F)
  * (the first edge end point became the second edge start point and vice-versa). It is used to fix crossings;
  * for instance, it can change:
+ *
+ * <pre>{@code
  * ... -> A B <- ...
  * ....... x .......
  * ... <- C D -> ...
+ * }</pre>
  *
  * to
  *
+ * <pre>{@code
  * ... -> A -> B -> ...
  * ... <- C <- D <- ...
+ * }</pre>
  *
  * Note the sub-path D...B was reversed. The 2-opt works be reversing the path between the two edges being removed.
- *
+ * <p>
  * When the edges are assigned to different entities, it results in a tail swap.
  * For instance, let r1 = [A, B, C, D], and r2 = [E, F, G, H]. Doing a
  * 2-opt on (B, C) + (F, G) will result in r1 = [A, B, G, H] and r2 = [E, F, C, D].
  *
  * @param <Solution_>
  */
-final class TwoOptListMove<Solution_> extends AbstractMove<Solution_> {
+public final class TwoOptListMove<Solution_> extends AbstractMove<Solution_> {
     private final ListVariableDescriptor<Solution_> variableDescriptor;
     private final Object firstEntity;
     private final Object secondEntity;
@@ -192,6 +197,19 @@ private void doSublistReversal(ScoreDirector<Solution_> scoreDirector) {
 
     @Override
     public boolean isMoveDoable(ScoreDirector<Solution_> scoreDirector) {
+        if (firstEntity == secondEntity) {
+            if (shift != 0) {
+                // A shift will rotate the entire list, changing the visiting order
+                return true;
+            }
+            int chainLength = Math.abs(secondEdgeEndpoint - firstEdgeEndpoint);
+
+            // The chain flipped by a K-Opt only changes if there are at least 2 values
+            // in the chain
+            return chainLength >= 2;
+        }
+        // This is a tail-swap move otherwise, which always changes at least one element
+        // (the element where the tail begins for each entity)
         return true;
     }
 

core/core-impl/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMoveTest.java

@@ -52,6 +52,72 @@ void doMove() {
         assertThat(e1.getValueList()).containsExactly(v1, v2, v5, v4, v3, v6, v7, v8);
     }
 
+    @Test
+    void isMoveDoable() {
+        TestdataListValue v1 = new TestdataListValue("1");
+        TestdataListValue v2 = new TestdataListValue("2");
+        TestdataListValue v3 = new TestdataListValue("3");
+        TestdataListValue v4 = new TestdataListValue("4");
+        TestdataListValue v5 = new TestdataListValue("5");
+        TestdataListValue v6 = new TestdataListValue("6");
+        TestdataListValue v7 = new TestdataListValue("7");
+        TestdataListValue v8 = new TestdataListValue("8");
+        TestdataListEntity e1 = TestdataListEntity.createWithValues("e1", v1, v2, v5, v4, v3, v6, v7, v8);
+
+        // 2-Opt((v2, v5), (v3, v6))
+        TwoOptListMove<TestdataListSolution> move = new TwoOptListMove<>(variableDescriptor,
+                e1, e1, 2, 5);
+        assertThat(move.isMoveDoable(scoreDirector)).isTrue();
+
+        // 2-Opt((v2, v3), (v2, v3))
+        move = new TwoOptListMove<>(variableDescriptor,
+                e1, e1, 2, 2);
+        assertThat(move.isMoveDoable(scoreDirector)).isFalse();
+
+        // 2-Opt((v2, v3), (v3, v4))
+        move = new TwoOptListMove<>(variableDescriptor,
+                e1, e1, 2, 3);
+        assertThat(move.isMoveDoable(scoreDirector)).isFalse();
+
+        // 2-Opt((v2, v3), (v4, v5))
+        move = new TwoOptListMove<>(variableDescriptor,
+                e1, e1, 2, 4);
+        assertThat(move.isMoveDoable(scoreDirector)).isTrue();
+
+        // 2-Opt((v2, v3), (v1, v2))
+        move = new TwoOptListMove<>(variableDescriptor,
+                e1, e1, 2, 1);
+        assertThat(move.isMoveDoable(scoreDirector)).isTrue();
+    }
+
+    @Test
+    void isMoveDoableTailSwap() {
+        TestdataListValue v1 = new TestdataListValue("1");
+        TestdataListValue v2 = new TestdataListValue("2");
+        TestdataListValue v3 = new TestdataListValue("3");
+        TestdataListValue v4 = new TestdataListValue("4");
+        TestdataListValue v5 = new TestdataListValue("5");
+        TestdataListValue v6 = new TestdataListValue("6");
+        TestdataListValue v7 = new TestdataListValue("7");
+        TestdataListValue v8 = new TestdataListValue("8");
+        TestdataListValue v9 = new TestdataListValue("9");
+        TestdataListEntity e1 = TestdataListEntity.createWithValues("e1", v1, v2, v3, v4);
+        TestdataListEntity e2 = TestdataListEntity.createWithValues("e2", v5, v6, v7, v8, v9);
+
+        // 2-Opt((v2, v3), (v6, v7))
+        TwoOptListMove<TestdataListSolution> move = new TwoOptListMove<>(variableDescriptor,
+                e1, e2, 2, 2);
+        assertThat(move.isMoveDoable(scoreDirector)).isTrue();
+
+        move = new TwoOptListMove<>(variableDescriptor,
+                e1, e2, 1, 2);
+        assertThat(move.isMoveDoable(scoreDirector)).isTrue();
+
+        move = new TwoOptListMove<>(variableDescriptor,
+                e1, e2, 2, 1);
+        assertThat(move.isMoveDoable(scoreDirector)).isTrue();
+    }
+
     @Test
     void doTailSwap() {
         TestdataListValue v1 = new TestdataListValue("1");