From a8f78744f51d68c867f35251f52613431738cd08 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Tue, 12 Dec 2023 12:27:12 -0500 Subject: [PATCH 1/2] fix: filter trivial 2-opt moves and make KOptListMove public --- .../move/generic/list/kopt/KOptListMove.java | 2 +- .../generic/list/kopt/TwoOptListMove.java | 15 ++++- .../generic/list/kopt/TwoOptListMoveTest.java | 66 +++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/KOptListMove.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/KOptListMove.java index c400a7b83b..39c2995368 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/KOptListMove.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/KOptListMove.java @@ -16,7 +16,7 @@ /** * @param the solution type, the class with the {@link PlanningSolution} annotation */ -final class KOptListMove extends AbstractMove { +public final class KOptListMove extends AbstractMove { private final ListVariableDescriptor listVariableDescriptor; private final KOptDescriptor descriptor; diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMove.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMove.java index d03f3e26b7..bf4b3e0eea 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMove.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMove.java @@ -36,7 +36,7 @@ * * @param */ -final class TwoOptListMove extends AbstractMove { +public final class TwoOptListMove extends AbstractMove { private final ListVariableDescriptor variableDescriptor; private final Object firstEntity; private final Object secondEntity; @@ -192,6 +192,19 @@ private void doSublistReversal(ScoreDirector scoreDirector) { @Override public boolean isMoveDoable(ScoreDirector 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; } diff --git a/core/core-impl/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMoveTest.java b/core/core-impl/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMoveTest.java index e18c9f0c9d..d604047f40 100644 --- a/core/core-impl/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMoveTest.java +++ b/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 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 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"); From d1db8d12e9cca1d931ae331f7d1d2d51c0e70dd2 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Tue, 12 Dec 2023 13:14:00 -0500 Subject: [PATCH 2/2] fix: fix Javadoc errors in TwoOptListMove --- .../selector/move/generic/list/kopt/KOptListMove.java | 4 ++-- .../selector/move/generic/list/kopt/TwoOptListMove.java | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/KOptListMove.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/KOptListMove.java index 39c2995368..ae1fccdbd1 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/KOptListMove.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/KOptListMove.java @@ -27,7 +27,7 @@ public final class KOptListMove extends AbstractMove { private final int[] newEndIndices; private final Object[] originalEntities; - public KOptListMove(ListVariableDescriptor listVariableDescriptor, + KOptListMove(ListVariableDescriptor listVariableDescriptor, SingletonInverseVariableSupply inverseVariableSupply, KOptDescriptor descriptor, List equivalent2Opts, @@ -59,7 +59,7 @@ public KOptListMove(ListVariableDescriptor listVariableDescriptor, } } - public KOptListMove(ListVariableDescriptor listVariableDescriptor, + KOptListMove(ListVariableDescriptor listVariableDescriptor, KOptDescriptor descriptor, List equivalent2Opts, MultipleDelegateList combinedList, diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMove.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMove.java index bf4b3e0eea..a70e71c512 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMove.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/list/kopt/TwoOptListMove.java @@ -19,17 +19,22 @@ * [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: + * + * 
{@code
  * ... -> A B <- ...
  * ....... x .......
  * ... <- C D -> ...
+ * }
* * to * + * 
{@code
  * ... -> A -> B -> ...
  * ... <- C <- D <- ...
+ * }
* * Note the sub-path D...B was reversed. The 2-opt works be reversing the path between the two edges being removed. - * + *

* 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].