Skip to content

Commit 5e7a8cf

Browse files
triceoclaude
andcommitted
fix: reject tabu size 0; validate VALUE/MOVE_TABU configs; remove dead else-if
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 93862a6 commit 5e7a8cf

5 files changed

Lines changed: 39 additions & 14 deletions

File tree

core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/AcceptorFactory.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public Acceptor<Solution_> buildAcceptor(HeuristicConfigPolicy<Solution_> config
5454
.collect(Collectors.toList());
5555

5656
if (acceptorList.size() == 1) {
57-
return acceptorList.get(0);
57+
return acceptorList.getFirst();
5858
} else if (acceptorList.size() > 1) {
5959
return new CompositeAcceptor<>(acceptorList);
6060
} else {
@@ -134,14 +134,12 @@ private Optional<EntityTabuAcceptor<Solution_>> buildEntityTabuAcceptor(Heuristi
134134
private Optional<ValueTabuAcceptor<Solution_>> buildValueTabuAcceptor(HeuristicConfigPolicy<Solution_> configPolicy) {
135135
if (acceptorTypeListsContainsAcceptorType(AcceptorType.VALUE_TABU)
136136
|| acceptorConfig.getValueTabuSize() != null || acceptorConfig.getFadingValueTabuSize() != null) {
137-
var acceptor = new ValueTabuAcceptor<Solution_>(configPolicy.getLogIndentation());
138-
if (acceptorConfig.getValueTabuSize() != null) {
139-
acceptor.setTabuSizeStrategy(new FixedTabuSizeStrategy<>(acceptorConfig.getValueTabuSize()));
137+
if (acceptorConfig.getValueTabuSize() == null && acceptorConfig.getFadingValueTabuSize() == null) {
138+
throw new IllegalArgumentException(
139+
"The acceptorType (%s) requires either valueTabuSize or fadingValueTabuSize to be configured."
140+
.formatted(AcceptorType.VALUE_TABU));
140141
}
141-
if (acceptorConfig.getFadingValueTabuSize() != null) {
142-
acceptor.setFadingTabuSizeStrategy(new FixedTabuSizeStrategy<>(acceptorConfig.getFadingValueTabuSize()));
143-
}
144-
142+
var acceptor = new ValueTabuAcceptor<Solution_>(configPolicy.getLogIndentation());
145143
if (acceptorConfig.getValueTabuSize() != null) {
146144
acceptor.setTabuSizeStrategy(new FixedTabuSizeStrategy<>(acceptorConfig.getValueTabuSize()));
147145
}
@@ -159,6 +157,11 @@ private Optional<ValueTabuAcceptor<Solution_>> buildValueTabuAcceptor(HeuristicC
159157
private Optional<MoveTabuAcceptor<Solution_>> buildMoveTabuAcceptor(HeuristicConfigPolicy<Solution_> configPolicy) {
160158
if (acceptorTypeListsContainsAcceptorType(AcceptorType.MOVE_TABU)
161159
|| acceptorConfig.getMoveTabuSize() != null || acceptorConfig.getFadingMoveTabuSize() != null) {
160+
if (acceptorConfig.getMoveTabuSize() == null && acceptorConfig.getFadingMoveTabuSize() == null) {
161+
throw new IllegalArgumentException(
162+
"The acceptorType (%s) requires either moveTabuSize or fadingMoveTabuSize to be configured."
163+
.formatted(AcceptorType.MOVE_TABU));
164+
}
162165
var acceptor = new MoveTabuAcceptor<Solution_>(configPolicy.getLogIndentation());
163166
if (acceptorConfig.getMoveTabuSize() != null) {
164167
acceptor.setTabuSizeStrategy(new FixedTabuSizeStrategy<>(acceptorConfig.getMoveTabuSize()));

core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/AbstractTabuAcceptor.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,6 @@ public boolean isAccepted(LocalSearchMoveScope<Solution_> moveScope) {
141141
logger.trace("{} Proposed move ({}) is tabu and is therefore not accepted.",
142142
logIndentation, moveScope.getMove());
143143
return false;
144-
} else if (workingFadingTabuSize == 0) {
145-
// No comment as the move is not tabu; the tabu list needs to be adjusted.
146-
return true;
147144
}
148145
// From this point, we are guaranteed to be in a fading tabu.
149146
var acceptChance = calculateFadingTabuAcceptChance(tabuStepCount - workingTabuSize);

core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/size/FixedTabuSizeStrategy.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ public final class FixedTabuSizeStrategy<Solution_> extends AbstractTabuSizeStra
88

99
public FixedTabuSizeStrategy(int tabuSize) {
1010
this.tabuSize = tabuSize;
11-
if (tabuSize < 0) {
12-
throw new IllegalArgumentException("The tabuSize (" + tabuSize
13-
+ ") cannot be negative.");
11+
if (tabuSize < 1) {
12+
throw new IllegalArgumentException("The tabuSize (%d) must be at least 1."
13+
.formatted(tabuSize));
1414
}
1515
}
1616

core/src/test/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/AcceptorFactoryTest.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,22 @@ <Solution_> void diversifiedLateAcceptanceAcceptor() {
110110
AcceptorFactory<Solution_> badAcceptorFactory = AcceptorFactory.create(localSearchAcceptorConfig);
111111
assertThatIllegalStateException().isThrownBy(() -> badAcceptorFactory.buildAcceptor(heuristicConfigPolicy));
112112
}
113+
114+
@Test
115+
<Solution_> void valueTabuWithoutSizes_throwsException() {
116+
var config = new LocalSearchAcceptorConfig()
117+
.withAcceptorTypeList(List.of(AcceptorType.VALUE_TABU));
118+
var factory = AcceptorFactory.create(config);
119+
assertThatIllegalArgumentException()
120+
.isThrownBy(() -> factory.buildAcceptor(mock(HeuristicConfigPolicy.class)));
121+
}
122+
123+
@Test
124+
<Solution_> void moveTabuWithoutSizes_throwsException() {
125+
var config = new LocalSearchAcceptorConfig()
126+
.withAcceptorTypeList(List.of(AcceptorType.MOVE_TABU));
127+
var factory = AcceptorFactory.create(config);
128+
assertThatIllegalArgumentException()
129+
.isThrownBy(() -> factory.buildAcceptor(mock(HeuristicConfigPolicy.class)));
130+
}
113131
}

core/src/test/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/size/FixedTabuSizeStrategyTest.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package ai.timefold.solver.core.impl.localsearch.decider.acceptor.tabu.size;
22

33
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
45
import static org.mockito.Mockito.mock;
56

67
import ai.timefold.solver.core.impl.localsearch.scope.LocalSearchStepScope;
@@ -16,4 +17,10 @@ void tabuSize() {
1617
assertThat(new FixedTabuSizeStrategy(17).determineTabuSize(stepScope)).isEqualTo(17);
1718
}
1819

20+
@Test
21+
void invalidTabuSize() {
22+
assertThatIllegalArgumentException().isThrownBy(() -> new FixedTabuSizeStrategy<>(0));
23+
assertThatIllegalArgumentException().isThrownBy(() -> new FixedTabuSizeStrategy<>(-1));
24+
}
25+
1926
}

0 commit comments

Comments
 (0)