From 4cbb5aa4b0de07865f3e9efd131ad8704731f582 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Wed, 15 Jan 2025 15:27:35 -0500 Subject: [PATCH] chore: Return NaN instead of throwing an exception on harder level change --- .../AdaptiveTerminationConfig.java | 2 +- .../termination/AdaptiveScoreRingBuffer.java | 2 +- .../termination/AdaptiveTermination.java | 76 +++++++++---------- 3 files changed, 36 insertions(+), 44 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/config/solver/termination/AdaptiveTerminationConfig.java b/core/src/main/java/ai/timefold/solver/core/config/solver/termination/AdaptiveTerminationConfig.java index f6ea89be36..8754bf2b5f 100644 --- a/core/src/main/java/ai/timefold/solver/core/config/solver/termination/AdaptiveTerminationConfig.java +++ b/core/src/main/java/ai/timefold/solver/core/config/solver/termination/AdaptiveTerminationConfig.java @@ -164,7 +164,7 @@ private void inheritGracePeriod(@NonNull AdaptiveTerminationConfig parent) { @Override public void visitReferencedClasses(@NonNull Consumer> classVisitor) { - + // intentionally empty - no classes to visit } /** Check whether any gracePeriod... is non-null. */ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AdaptiveScoreRingBuffer.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AdaptiveScoreRingBuffer.java index 94ee93fefe..2e23b33364 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AdaptiveScoreRingBuffer.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AdaptiveScoreRingBuffer.java @@ -9,7 +9,7 @@ import org.jspecify.annotations.Nullable; final class AdaptiveScoreRingBuffer> { - private final static int DEFAULT_CAPACITY = 4096; + private static final int DEFAULT_CAPACITY = 4096; int readIndex; int writeIndex; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AdaptiveTermination.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AdaptiveTermination.java index 589aedf4a7..44679f545b 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AdaptiveTermination.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/termination/AdaptiveTermination.java @@ -37,29 +37,18 @@ public double getMinimumImprovementRatio() { } /** - * An exception that is thrown to signal a hard score improvement. - * This is done since: - *

- *

    - *
  • Hard score improvements are significantly rarer than soft score improvements. - *
  • Allows us to directly return a double in a method instead of using a - * carrier type/needing to box it so we can use null as a special value. - *
  • Avoid branching in code on the hot path. - *
- *

- * This exception does not fill the stack trace to improve performance; - * it is a signal that should always be caught - * and handled appropriately. + * Returns the improvement in the softest level between the prior + * and current best scores as a double, or {@link Double#NaN} if + * there is a difference in any of their other levels. + * + * @param start the prior best score + * @param end the current best score + * @return the softest level difference between end and start, or + * {@link Double#NaN} if a harder level changed + * @param The score type */ - private static final class HardLevelImprovedSignal extends Exception { - @Override - public synchronized Throwable fillInStackTrace() { - return this; - } - } - - private static > double softImprovement(@NonNull Score_ start, - @NonNull Score_ end) throws HardLevelImprovedSignal { + private static > double softImprovementOrNaNForHarderChange(@NonNull Score_ start, + @NonNull Score_ end) { if (start.equals(end)) { // optimization: since most of the time the score the same, // we can use equals to avoid creating double arrays in the @@ -70,7 +59,7 @@ private static > double softImprovement(@NonNull Sc var softestLevel = scoreDiffs.length - 1; for (int i = 0; i < softestLevel; i++) { if (scoreDiffs[i] != 0.0) { - throw new HardLevelImprovedSignal(); + return Double.NaN; } } return scoreDiffs[softestLevel]; @@ -96,28 +85,31 @@ private void resetGracePeriod(long currentTime, Score_ startingScore) { } public boolean isTerminated(long currentTime, Score_ endScore) { - try { - if (isGracePeriodActive) { - // first score in scoresByTime = first score in grace period window - var endpointDiff = softImprovement(scoresByTime.peekFirst(), endScore); - var timeElapsedNanos = currentTime - gracePeriodStartTimeNanos; - if (timeElapsedNanos >= gracePeriodNanos) { - // grace period over, record the reference diff - isGracePeriodActive = false; - gracePeriodSoftestImprovementDouble = endpointDiff; - return endpointDiff == 0.0; - } + if (isGracePeriodActive) { + // first score in scoresByTime = first score in grace period window + var endpointDiff = softImprovementOrNaNForHarderChange(scoresByTime.peekFirst(), endScore); + if (Double.isNaN(endpointDiff)) { + resetGracePeriod(currentTime, endScore); return false; } + var timeElapsedNanos = currentTime - gracePeriodStartTimeNanos; + if (timeElapsedNanos >= gracePeriodNanos) { + // grace period over, record the reference diff + isGracePeriodActive = false; + gracePeriodSoftestImprovementDouble = endpointDiff; + return endpointDiff == 0.0; + } + return false; + } - var startScore = scoresByTime.pollLatestScoreBeforeTimeAndClearPrior(currentTime - gracePeriodNanos); - var scoreDiff = softImprovement(startScore, endScore); - - return scoreDiff / gracePeriodSoftestImprovementDouble < minimumImprovementRatio; - } catch (HardLevelImprovedSignal signal) { + var startScore = scoresByTime.pollLatestScoreBeforeTimeAndClearPrior(currentTime - gracePeriodNanos); + var scoreDiff = softImprovementOrNaNForHarderChange(startScore, endScore); + if (Double.isNaN(scoreDiff)) { resetGracePeriod(currentTime, endScore); return false; } + + return scoreDiff / gracePeriodSoftestImprovementDouble < minimumImprovementRatio; } @Override @@ -149,12 +141,12 @@ public Termination createChildThreadTermination(SolverScope phaseScope) { - + // intentionally empty - no work to do } @Override public void stepStarted(AbstractStepScope stepScope) { - + // intentionally empty - no work to do } @Override @@ -164,7 +156,7 @@ public void stepEnded(AbstractStepScope stepScope) { @Override public void phaseEnded(AbstractPhaseScope phaseScope) { - + // intentionally empty - no work to do } @Override