Repository navigation
Conversation
nlsolve! accepts a Newton solve after one iteration when θ / (1 - θ) * ndz < κ with θ = prev_θ from an earlier solve. Only solves with two or more iterations update prev_θ, and nothing reset it when W was rebuilt, so a small rate measured once could let later solves stop after a single iteration regardless of their W. With NordsieckBDF on a stiff square-root nonlinearity that left an error flipping sign every step, which passed the error test but stalled the step size. Reset prev_θ to one when a Newton solve gets a new W, as CVODE does (crate = ONE in cvNlsLSetup). Functional and Anderson iteration report a new W on every solve and always_new rebuilds W every iteration, so they keep their rate. Add the reproducer from the issue as a regression test, above the BackTracking loop that errors on master (SciML#4681) so that it runs. The fix lowers nf for both plain and BackTracking Trapezoid on Lorenz, but widens the gap between them from 3 to 27 of ~370k evaluations, so that budget becomes relative (0.1%) instead of an absolute 20. Fixes SciML#4983 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011fkoQWACM13p7zeDc6Rwcj
PR: #4986 Reviewed head: 8df2490 Risk assessment
Review reportVERDICT: CHANGES Riskhigh (H2: shared Newton stopping behavior changes default FBDF solves, including automatic stiff selection; H4: an existing test bound is relaxed without a derivation). Blocking findingsThe head fixes both accuracy and step-count failures in the pit reproduction. Two test issues remain:
Non-blocking findings
What I ranAI reviewer: Codex, model Commands set the requested
The new step-count and accuracy assertions fail on the base reproduction and pass on head. The FBDF results also demonstrate default behavior changes and increased RHS work on an already accurate path; no wall-time benchmark was run.
What I did not verify
AUTHOR_GRADE: {"task":"bugfix","score":4,"difficulty":4,"rounds":1,"author_harness":"unknown","author_model":"unknown","note":"Verified the pit accuracy and step-count fix, broad analytic cases, and default-FBDF parameter gradients. Core failure patterns also reproduce with the base driver. The new test's dependency range and the relaxed evaluation-count guard still need correction or justification."} Head link: 8df2490 🤖 Posted by an AI agent — harness: codex-cli 0.164.0 · model: gpt-6.1-sol |
Changes from review round 1 of SciML#4986. A linear function gets a new W on every change of γdt, so the restart forced a second Newton iteration on nearly every step there: 111 to 190 iterations for KenCarp4 on a split problem with a linear implicit part, 99 to 166 for FBDF on a linear operator. Its convergence rate does not depend on the state, so it now keeps the rate, and those counts are back at their values on master. A new test asserts fewer than 1.5 iterations per implicit stage on the IMEX case. The pit test now checks the balance itself, total outflow equal to the inflow within 1%, instead of a level window that also accepted h = 0. Its tests need OrdinaryDiffEqBDF 2.4.2, the first release with NordsieckBDF. The BackTracking guard compares Newton iteration counts, because stats.nf leaves out the line search's own evaluations. Its gap to plain Newton is noise of about ±1.5% from two diverged step sequences, on master too, so the 0.1% budget failed on Orego in the downgrade job. The budget is now 10%; shortening every step by 10% costs 70% to 100%. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011fkoQWACM13p7zeDc6Rwcj
Reply to the round 1 review of head 8df2490. New head: 3b73913. It merges master at 535e41f (clean; brings in #4794 and #4994) and adds one commit with the changes below. The PR description is updated to match. Blocking findings[P2] Relaxed regression guard: valid, and the 0.1% budget was wrong. It failed in this PR's own downgrade job on Orego (
The comment above the assertion now states the metric and these figures. #4894 edits the same assertion (0.1% for Orego, [P2] Dependency bound: valid. The downgrade job resolved Non-blocking findings
Change not requested by the review: linear functions keep their rateChecking H2 turned up a regression in the fix as pushed. For a linear function
The restart now skips Two cases I tested and deliberately did not exempt or include:
Risk H2: measured costHead against base, Julia 1.13.4, in-repo sibling packages, no bounds checking, interleaved runs, minimum of repeated solves:
Julia 1.13.4 was released during this round. Everything from the latest stacks was measured on 1.13.3 first and rerun on 1.13.4. Solver statistics and end-point errors are identical in all 546 solves of the count benchmark (182 cases under base, old head and new head) and in every probe quoted here. The time ratios on 1.13.3 were ×1.018, ×1.076, ×1.037 and ×0.997.
Two further observations:
CI on the reviewed headOf the 13 failing checks, one was caused by this PR:
To expect on the next run: once CI's Julia 1 jobs pick up 1.13.4, Enzyme-based tests abort there regardless of this PR. Enzyme 0.13.219, the latest release, hits What I ranAll on the new head's tree, one Julia process per test file unless noted.
What I did not verify
Author attribution: harness Claude Code. This round's changes and this reply: model Fable 5.2 ( 🤖 Posted by an AI agent on behalf of the PR author — harness: Claude Code · model: Fable 5.2 ( |
Fixes #4983.
nlsolve!accepts a Newton solve after one iteration whenθ / (1 - θ) * ndz < κ, withθ = prev_θremembered from an earlier solve. Only solves with two or more iterations updateprev_θ, and nothing made it expire. This PR resetsprev_θto one when a Newton solve gets a new W, as CVODE does (crate = ONEincvNlsLSetup), so the rate is measured again.What goes wrong on master
In the issue's reproducer (
NordsieckBDF, a stiff square-root nonlinearity, RMS norm over N states) the Jacobian is evaluated once, at the first step, and never again (njacs = 1over 2793 steps). A rate of 0.024 is measured at step 6. The state then enters the steep part of the square root, where that Jacobian is far off, but every later step still stops after one iteration on the remembered rate. Each step overshoots, the error flips sign, and the step size stalls.With the restart the rate is measured again after every W rebuild. In this run the solve at step 7 then no longer passes on its first iteration and gets a fresh Jacobian, and the stall never develops (4 Jacobians over 16 steps).
Accepted steps and final level (converged: −0.05 mm):
The same stall occurs matrix-free (
linsolve = KrylovJL_GMRES(), N = 8000: 3584 steps on master, 20 here), so it is not only a stale stored Jacobian.Where the restart applies
Only for
NLNewtonon a new W, and not in these cases:get_new_W! == trueon every solve, so a restart there would floor their second-iteration θ at 0.3 every time.islinearfunction). W is rebuilt on every change of γdt there, so a restart forced a second iteration on nearly every step: 111 to 190 Newton iterations for IMEXKenCarp4with a linear implicit part, 99 to 166 forFBDFon a linear operator. For a linear function the rate does not depend on the state, so the remembered rate stays valid. With the exemption these counts equal master's.always_new = true, where every W is new. This is a known limitation:NordsieckBDF(nlsolve = NLNewton(always_new = true))at N = 8000 still takes 3607 steps, as on master. Restarting there too fixes it but costs ×1.38 Newton iterations on standard problems, each with a Jacobian and a factorization.The restart bounds how long a stale rate can be used, to the next W rebuild. It does not remove staleness between rebuilds. CVODE additionally forces a setup every 20 steps.
Cost
This PR against master, Julia 1.13.4, minimum of repeated solves:
adaptive = false)Short solves that rebuild W on nearly every step are at the high end, for example
FBDFon the reproducer: 114 to 197 RHS calls over 24 steps. Three other ways of expiring the rate (restart only when the Jacobian is not current, CVODE'smin(1, crate)first-iteration test, a Hairer-style fading rate) also fix the reproducer and cost within two percentage points of this rule on the stiff set.One side effect: at tolerances near 1e-12 the restart adds iterations whose residual is at the rounding floor, where the finite-difference slope of the
relaxline search is noise.Trapezoid(nlsolve = NLNewton(relax = BackTracking()))then hits #4681 at more nearby tolerances (Orego: 7 of 8 instead of 3 of 8). At exactly 1e-12 nothing changes.Tests
In
newton_tests.jl, above the Lorenz/Orego BackTracking loop, which errors on master (#4681):KenCarp4on a linear operator (1.19 here and on master, 2.0 without the exemption).nf <= 1.1 * nf_plaininstead of+ 20.stats.nfleaves out the line search's own evaluations, so this compares Newton iteration counts. The gap is noise from two diverged step sequences, about ±1.5% over nearby tolerances on master as well, while shortening every step by 10% costs 70% to 100%. Report non-descent BackTracking directions as a failed Newton step instead of crashing #4894 edits the same assertion.OrdinaryDiffEqBDF2.4.2, the first release withNordsieckBDF.Verification, detailed in the reply to the round 1 review below:
--check-bounds=yes): 30 of 32 files pass.newton_tests.jl(Newton BackTracking test fails on master since 498c765ccc (calc_dt_propose! dt rounding) #4681) andnsa_noinit_tests.jl(4MaxIters) fail identically on master.EnzymeAD/Enzyme.jl#4051); its other 18 files pass.🤖 Generated with Claude Code
https://claude.ai/code/session_011fkoQWACM13p7zeDc6Rwcj