Repository navigation
Use the RadauIIA3 tolerance exponent in place as out of place - #4934
singhharsh1708 wants to merge 3 commits into
Conversation
Independent review (Codex CLI 0.163.0, model gpt-6.1-sol, reasoning effort high): MERGE, risk medium (M2). Reviewed head: Full reviewVERDICT: MERGE Riskmedium (M2: adaptive step grids and numerical results change on an already-working peripheral solver path.) Risk assessment
Blocking findingsNone. Non-blocking findingsNone. What I ranCommands below show the supplied Julia launcher relative to each worktree.
What I did not verify
AUTHOR_GRADE: {"task":"bugfix","score":5,"difficulty":2,"rounds":1,"author_harness":"unknown","author_model":"unknown","note":"Focused tolerance-exponent correction with a regression that fails on base and passes on head. Retained independent reference tests pass; analytic scalar checks confirm accuracy, while a vector-tolerance initialization limitation reproduces on both revisions."} 🤖 Posted by an AI agent · harness: Claude Code · model: claude-opus-5-5 (coordinating gpt-6.1-sol reviewers) |
PR: #4934 Reviewed head: 78a3d07 Risk assessment
Review reportVERDICT: CHANGES Riskmedium (M2: the tolerance scaling can change numerical results and accepted-step sequences on existing in-place RadauIIA3 calls; the default solver is unaffected by this diff).
Blocking findings
Non-blocking findings
What I ranAI reviewer: codex-cli 0.164.0, model gpt-6.1-sol. The Session field is the actual local session ID; no session URL is available. No author rating, PR body, comments, reviews, or sibling review artifacts were read.
What I did not verifyNo numerical assertion executed. Prepared but unrun probes covered the added Van der Pol regression with completion checks; analytic linear and stiff systems with scalar/component tolerances and Float32/Float64; nonlinear FIRK Core, QA, downstream suites, performance impact, and other supported Julia versions remain unverified. The numerical probes' existence is not evidence that they pass. No checkout edits, commits, pushes, GitHub posts, or merges were made. AUTHOR_GRADE: {"task":"bugfix","score":3,"difficulty":2,"rounds":1,"author_harness":"unknown","author_model":"unknown","note":"The two exponent edits match the out-of-place implementation and a regression assertion is present. Environment resolution and loading the checkout passed, but resource limits prevented numerical validation; the supplied downstream CI regressions remain unresolved."} Head link: 78a3d07 🤖 Posted by an AI agent — harness: codex-cli 0.164.0 · model: gpt-6.1-sol |
|
Added successful-retcode and final-time checks for both the in-place and out-of-place solves. A vector-tolerance case can't be added here: in-place RadauIIA3 with a vector reltol throws a MethodError at init on master, which #4920 fixes. |
In-place
RadauIIA3now sets its internal tolerance toreltol^(3/4) / 10, as the out-of-placeperform_step!and_ode_addsteps!already do.initialize!(integrator, ::RadauIIA3Cache)used 2/3, the RadauIIA5 value; Hairer's(s + 1) / (2s), which RadauIIA5 (2/3), RadauIIA9 (3/5) and AdaptiveRadau follow on both paths, gives 3/4 for two stages.On Van der Pol with μ = 10 (tspan 0 to 10), in place now takes 2234, 12195 and 68416 steps at reltol 1e-4, 1e-6 and 1e-8, against 2192, 12233 and 68436 out of place; master's in-place counts were 1531, 6865 and 31959. In-place solves at a given reltol therefore take more steps than before, matching what out of place already did.
The new testset requires the two step counts to agree within 5% at 1e-4 and fails on master (a difference of 661 against a bound of 110). FIRK Core 192/192; QA has the same 5 broken allocation tests as master.
The change sits next to #4920's edit of the same block, so whichever of the two lands second needs a trivial rebase.
Fixes #4928
AI Disclosure: Claude assisted with this change.