Skip to content

Commit 2255f55

Browse files
zepfredzepfred
committed
fix: improve strategy
1 parent 03e5942 commit 2255f55

2 files changed

Lines changed: 41 additions & 31 deletions

File tree

‎core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/decorator/FilteringMoveSelector.java‎

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
public final class FilteringMoveSelector<Solution_> extends AbstractMoveSelector<Solution_> {
1515

16+
private static final long BAIL_OUT_MULTIPLIER = 10L;
17+
1618
public static <Solution_> FilteringMoveSelector<Solution_> of(MoveSelector<Solution_> moveSelector,
1719
SelectionFilter<Solution_, Move<Solution_>> filter) {
1820
if (moveSelector instanceof FilteringMoveSelector<Solution_> filteringMoveSelector) {
@@ -75,6 +77,27 @@ public Iterator<Move<Solution_>> iterator() {
7577
return new JustInTimeFilteringMoveIterator(childMoveSelector.iterator(), determineBailOutSize(), phaseScope);
7678
}
7779

80+
private long determineBailOutSize() {
81+
if (!bailOutEnabled) {
82+
return -1L;
83+
}
84+
try {
85+
return childMoveSelector.getSize() * BAIL_OUT_MULTIPLIER;
86+
} catch (Exception ex) {
87+
// Some move selectors throw an exception when getSize() is called.
88+
// In this case, we choose to disregard it and pick a large-enough bail-out size anyway.
89+
// The ${bailOutSize+1}th move could in theory show up where previous ${bailOutSize} moves did not,
90+
// but we consider this to be an acceptable risk,
91+
// outweighed by the benefit of the solver never running into an endless loop.
92+
// The exception itself is swallowed, as it doesn't bring any useful information.
93+
long bailOutSize = Short.MAX_VALUE * BAIL_OUT_MULTIPLIER;
94+
logger.trace(
95+
" Never-ending move selector ({}) failed to provide size, choosing a bail-out size of ({}) attempts.",
96+
childMoveSelector, bailOutSize);
97+
return bailOutSize;
98+
}
99+
}
100+
78101
private class JustInTimeFilteringMoveIterator extends UpcomingSelectionIterator<Move<Solution_>> {
79102

80103
private final Iterator<Move<Solution_>> childMoveIterator;
@@ -94,6 +117,10 @@ public JustInTimeFilteringMoveIterator(Iterator<Move<Solution_>> childMoveIterat
94117
protected Move<Solution_> createUpcomingSelection() {
95118
Move<Solution_> next;
96119
long attemptsBeforeBailOut = bailOutSize;
120+
// To reduce the impact of checking for termination on each move,
121+
// we only check for termination
122+
// after filtering out a total number of moves equal to size(childMoveSelector).
123+
long attemptsBeforeCheckTermination = bailOutSize / BAIL_OUT_MULTIPLIER;
97124
do {
98125
if (!childMoveIterator.hasNext()) {
99126
return noUpcomingSelection();
@@ -104,13 +131,18 @@ protected Move<Solution_> createUpcomingSelection() {
104131
logger.trace("Bailing out of neverEnding selector ({}) after ({}) attempts to avoid infinite loop.",
105132
FilteringMoveSelector.this, bailOutSize);
106133
return noUpcomingSelection();
107-
} else if (termination != null && termination.isPhaseTerminated(phaseScope)) {
108-
logger.trace(
109-
"Bailing out of neverEnding selector ({}) because the termination setting has been triggered.",
110-
FilteringMoveSelector.this);
111-
return noUpcomingSelection();
134+
} else if (termination != null && attemptsBeforeCheckTermination <= 0L) {
135+
// Reset the counter
136+
attemptsBeforeCheckTermination = bailOutSize / BAIL_OUT_MULTIPLIER;
137+
if (termination.isPhaseTerminated(phaseScope)) {
138+
logger.trace(
139+
"Bailing out of neverEnding selector ({}) because the termination setting has been triggered.",
140+
FilteringMoveSelector.this);
141+
return noUpcomingSelection();
142+
}
112143
}
113144
attemptsBeforeBailOut--;
145+
attemptsBeforeCheckTermination--;
114146
}
115147
next = childMoveIterator.next();
116148
} while (!accept(scoreDirector, next));
@@ -119,27 +151,6 @@ protected Move<Solution_> createUpcomingSelection() {
119151

120152
}
121153

122-
private long determineBailOutSize() {
123-
if (!bailOutEnabled) {
124-
return -1L;
125-
}
126-
try {
127-
return childMoveSelector.getSize() * 10L;
128-
} catch (Exception ex) {
129-
// Some move selectors throw an exception when getSize() is called.
130-
// In this case, we choose to disregard it and pick a large-enough bail-out size anyway.
131-
// The ${bailOutSize+1}th move could in theory show up where previous ${bailOutSize} moves did not,
132-
// but we consider this to be an acceptable risk,
133-
// outweighed by the benefit of the solver never running into an endless loop.
134-
// The exception itself is swallowed, as it doesn't bring any useful information.
135-
long bailOutSize = Short.MAX_VALUE * 10L;
136-
logger.trace(
137-
" Never-ending move selector ({}) failed to provide size, choosing a bail-out size of ({}) attempts.",
138-
childMoveSelector, bailOutSize);
139-
return bailOutSize;
140-
}
141-
}
142-
143154
private boolean accept(ScoreDirector<Solution_> scoreDirector, Move<Solution_> move) {
144155
if (filter != null && !filter.accept(scoreDirector, move)) {
145156
logger.trace(" Move ({}) filtered out by a selection filter ({}).", move, filter);

‎core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/decorator/FilteringMoveSelectorTest.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,18 +52,17 @@ void bailOutByTermination() {
5252
var moveSelector = mock(MoveSelector.class);
5353
var termination = mock(BasicPlumbingTermination.class);
5454
var iterator = mock(Iterator.class);
55-
// We set the maximum value to force it to run many evaluations
56-
when(moveSelector.getSize()).thenReturn(Long.MAX_VALUE / 11);
55+
when(moveSelector.getSize()).thenReturn(100L);
5756
when(moveSelector.isNeverEnding()).thenReturn(true);
5857
when(moveSelector.iterator()).thenReturn(iterator);
5958
when(iterator.hasNext()).thenReturn(true);
6059
when(phaseScope.getTermination()).thenReturn(termination);
61-
when(termination.isPhaseTerminated(any(AbstractPhaseScope.class))).thenReturn(false, false, true);
60+
when(termination.isPhaseTerminated(any(AbstractPhaseScope.class))).thenReturn(false, true);
6261
var filteredMoveSelector = FilteringMoveSelector.of(moveSelector, (scoreDirector, selection) -> false);
6362
filteredMoveSelector.phaseStarted(phaseScope);
6463
assertThat(filteredMoveSelector.iterator().hasNext()).isFalse();
65-
// The termination returns true at the third call
66-
verify(iterator, times(2)).next();
64+
// The termination returns true at the second call, and only (100 * 10) * 30% calls are executed per check
65+
verify(iterator, times(200)).next();
6766
}
6867

6968
public void filter(SelectionCacheType cacheType, int timesCalled) {

0 commit comments

Comments
 (0)