Skip to content

fix: some terminations do not honor their contracts - #2801

Open
triceo wants to merge 8 commits into
TimefoldAI:mainfrom
triceo:fix-time-gradient
Open

triceo wants to merge 8 commits into
TimefoldAI:mainfrom
triceo:fix-time-gradient

Conversation

@triceo

@triceo triceo commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

We address the following issues:

  • Phase validation rejects reachable phases
  • A solver-level AND ignores its phase-only terminations
  • AND and OR return wrong values when no termination supports a time gradient
  • Diminished returns has no time gradient.
  • Diminished returns restarts its grace period late inside an AND
  • Simulated annealing keeps its starting temperature without a message

…that can end the phase

Simulated annealing cools by the time gradient of the phase termination.
- OR and AND composites now return -1.0 when no termination supports a time gradient,
  as the contract says. OR returned 0.0 and AND 1.0, so simulated annealing did not cool,
  or started at its minimum temperature, and a nested composite hid the gradient of the others.
- The solver bridge now includes the phase-only terminations of the solver termination,
  which can end the phase too. Diminished returns configured at solver level never reached the gradient.
- Diminished returns now estimates the time gradient: one sliding window is left
  after the time since its grace period started. This estimate was chosen from six candidates
  in an offline replay of diminished returns on quickstart runs.
- Without any termination that can estimate the time left, for example when only terminateEarly()
  stops the solver, simulated annealing keeps its starting temperature, as before, and logs a warning.

# Conflicts:
#	core/src/test/java/ai/timefold/solver/core/impl/solver/termination/DiminishedReturnsTerminationTest.java
@triceo
triceo marked this pull request as ready for review November 5, 2026 07:33
@triceo
triceo requested a review from a team November 5, 2026 07:33
@triceo
triceo requested a balanced review from Copilot and removed request for TomCools and rsynek November 5, 2026 07:33
@triceo triceo added this to the v2.9.0-rc-1 milestone Nov 5, 2026
@triceo triceo self-assigned this Nov 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The bridge understates the time gradient for mixed AND terminations, keeping simulated annealing near its starting temperature until the deadline; the gradient calculation needs correction and a mixed-AND regression test.

1 open issue
What changed in this PR

This PR improves simulated annealing cooling by correcting termination time gradients and adding a diminished-returns estimate.

Changes:

  • Return -1.0 when composite terminations cannot provide a time gradient.
  • Include phase-only terminations in the solver bridge’s gradient and estimate progress from diminished returns.
  • Warn when cooling is unavailable, add regression tests, and document behavior changes.
File Description
docs/​src/​modules/​ROOT/​pages/​upgrading-timefold-solver/​upgrade-to-latest.adoc Explains cooling changes and the new warning.
docs/​src/​modules/​ROOT/​pages/​optimization-algorithms/​local-search.adoc Documents termination requirements for cooling.
core/​src/​test/​java/​ai/​timefold/​solver/​core/​impl/​solver/​termination/​SolverBridgePhaseTerminationTest.java Tests phase-only gradients through the bridge.
core/​src/​test/​java/​ai/​timefold/​solver/​core/​impl/​solver/​termination/​OrCompositeTerminationTest.java Updates unsupported-gradient expectations.
core/​src/​test/​java/​ai/​timefold/​solver/​core/​impl/​solver/​termination/​DiminishedReturnsTerminationTest.java Tests gradient estimates and resets.
core/​src/​test/​java/​ai/​timefold/​solver/​core/​impl/​solver/​termination/​AndCompositeTerminationTest.java Tests unsupported and nested gradients.
core/​src/​test/​java/​ai/​timefold/​solver/​core/​impl/​localsearch/​decider/​acceptor/​simulatedannealing/​SimulatedAnnealingAcceptorTest.java Tests temperature without a supported gradient.
core/​src/​main/​java/​ai/​timefold/​solver/​core/​impl/​solver/​termination/​SolverBridgePhaseTermination.java Includes phase-only gradient estimates.
core/​src/​main/​java/​ai/​timefold/​solver/​core/​impl/​solver/​termination/​OrCompositeTermination.java Corrects the unsupported-gradient result.
core/​src/​main/​java/​ai/​timefold/​solver/​core/​impl/​solver/​termination/​DiminishedReturnsTermination.java Adds a sliding-window gradient estimate.
core/​src/​main/​java/​ai/​timefold/​solver/​core/​impl/​solver/​termination/​AndCompositeTermination.java Corrects unsupported-gradient handling.
core/​src/​main/​java/​ai/​timefold/​solver/​core/​impl/​solver/​termination/​AbstractCompositeTermination.java Combines solver and phase-only gradients.
core/​src/​main/​java/​ai/​timefold/​solver/​core/​impl/​localsearch/​decider/​acceptor/​simulatedannealing/​SimulatedAnnealingAcceptor.java Preserves starting temperature and warns when no gradient exists.

Review effort: Balanced

@triceo
triceo had a problem deploying to documentation (preview) November 6, 2026 09:29 — with GitHub Actions Failure

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Short-circuiting in solver-level AND termination leaves the diminished-returns grace period stale after harder-score changes, preventing simulated annealing from reheating and requiring a state-reset fix with regression coverage.

1 open issue
1 resolved since last review

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 6, 2026 09:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The AND termination short-circuits stateful diminished-returns updates until the time limit expires, corrupting its baseline and potentially ending a phase despite constant-rate score improvement.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

High severity Evaluate all termination conditions before combining results

core/​src/​main/​java/​ai/​timefold/​solver/​core/​impl/​solver/​termination/​AndCompositeTermination.java:65

For a solver-level AND of a time limit and diminished returns, this early return prevents diminished returns from updating its grace period until the time limit expires. With a 300-second limit, a 30-second window and a 0.5 minimum ratio, this records 300 seconds of improvement as the baseline and can incorrectly end the phase even when the score improves at a constant rate. Evaluate every child before returning the combined result so that stateful terminations keep their windows current.

Review effort: Balanced

The grace period restarted only inside isPhaseTerminated().
An AND skips that call after a termination that is not met,
so the restart came late, and simulated annealing did not heat up again.
Copilot AI balanced review requested due to automatic review settings November 6, 2026 09:39
@triceo triceo changed the title fix: give simulated annealing a time gradient fix: some terminations do not honor their contracts Nov 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Representative end-to-end quality benchmarks are needed because replaying prior runs cannot validate how the new cooling estimate affects annealing, diminished returns, and future improvements.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@triceo
triceo requested a review from zepfred November 6, 2026 09:43

This branch was successfully deployed

2 active deployments
documentation (preview) — 9e9419e4 Deployed Nov 6, 2026 by triceo via Build Documentation #858
internal — 9e9419e4 Deployed Nov 6, 2026 by triceo via approval_required #858
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants