Skip to content

Use the RadauIIA3 tolerance exponent in place as out of place - #4934

Open
singhharsh1708 wants to merge 3 commits into
SciML:masterfrom
singhharsh1708:radauiia3-inplace-rtol
Open

singhharsh1708 wants to merge 3 commits into
SciML:masterfrom
singhharsh1708:radauiia3-inplace-rtol

Conversation

@singhharsh1708

Copy link
Copy Markdown
Member

In-place RadauIIA3 now sets its internal tolerance to reltol^(3/4) / 10, as the out-of-place perform_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.

@singhharsh1708

Copy link
Copy Markdown
Member Author

🤖 Automated comment from an AI agent running as @singhharsh1708, not written or reviewed by the account holder.

Independent review (Codex CLI 0.163.0, model gpt-6.1-sol, reasoning effort high): MERGE, risk medium (M2). Reviewed head: 78a3d0718528285a54c9053721352f227507faa1.

Full review

VERDICT: MERGE
PR: #4934
Round: 1
Head: 78a3d07
Base: 45ff46a
Slot: 1
Reviewer: Codex CLI 0.163.0 / gpt-6.1-sol
Session: 01a19733-7eb9-7c42-aae5-362416a735da
Reviewed: 2026-11-01T13:08:10Z
Summary: Aligns adaptive in-place RadauIIA3 tolerance scaling with the out-of-place implementation. The regression fails on base and passes on head; independent accuracy checks found no introduced correctness defect.

Risk

medium (M2: adaptive step grids and numerical results change on an already-working peripheral solver path.)

Risk assessment

  • Risk: medium (M2: assessed from the diff before reading the description or comments; existing in-place RadauIIA3 solves take different steps.)
  • Blast radius: Adaptive in-place RadauIIA3 initialization, including scalar and vector tolerance transformations. No default solver selection or public API changes.
  • Evidence: Changed test file passes on head; copying it into base produces the intended regression failure. Retained adaptive tests compare against a separate RadauIIA5 implementation; analytic decay checks also pass for scalar tolerances. FIRK CI is green.
  • Verdict: MERGE
  • Round: 1
  • Blocking priorities: none
  • Merge: merge

Blocking findings

None.

Non-blocking findings

None.

What I ran

Commands below show the supplied Julia launcher relative to each worktree.

  • Git inspection, all exit 0:

    git rev-parse HEAD
    git merge-base origin/master HEAD
    git diff origin/master...HEAD
    git diff --check origin/master...HEAD
    git log -1 --format=%B
    git log -1 --no-merges --format=%B

    Head and merge-base matched the specified SHAs. No author harness/model trailers were present.

  • Live metadata and linked issue, both exit 0:

    gh pr view 4934 --repo SciML/OrdinaryDiffEq.jl --json state,isDraft,headRefOid,baseRefOid
    gh issue view 4928 --repo SciML/OrdinaryDiffEq.jl

    PR remained open at the specified head and base.

  • Head and base, Julia 1.13.1, sequentially through the supplied launcher:

    ../../jrun --project=lib/OrdinaryDiffEqFIRK -e 'using TestEnv; TestEnv.activate(); using Test, SafeTestsets; println("Julia ", VERSION); include("lib/OrdinaryDiffEqFIRK/test/ode_firk_tests.jl")'

    Head exit 0. Output tail:

    RadauIIA3 in place and out of place transform the tolerance alike | 1  1
    AdaptiveRadau Newton tolerance follows the stage count            | 8  8
    FIRK stiff member of an AutoSwitch composite (#4364)              | 10 10
    

    I copied the PR’s changed test file into the merge-base worktree and ran the same command there. Base exit 1, specifically at lib/OrdinaryDiffEqFIRK/test/ode_firk_tests.jl:285:

    Evaluated: 661 < 109.60000000000001
    Some tests did not pass: 0 passed, 1 failed, 0 errored, 0 broken.
    
  • Analytic probe on head and base:

    ../../jrun --project=lib/OrdinaryDiffEqFIRK ../../tmp/SciML__OrdinaryDiffEq.jl__4934/accuracy_probe.jl

    Checked decay equations with exact solution [exp(-t), exp(-100t)]. At scalar reltol=1e-4, both revisions succeeded: head in-place maximum sampled error 6.44e-7 with 177 accepted steps; base error 1.32e-6 with 123 steps. Out-of-place results were unchanged.

    Both probe runs exited 1 when reaching in-place vector tolerances:

    MethodError: no method matching Float64(::Vector{Float64})
    ... firk_caches.jl:114
    9 passed, 0 failed, 1 errored
    

    Confirmed by running: this failure occurs on both revisions inside LinearSolve initialization, before the changed initialization method.

  • Final git checks exited 0: head remained clean; base differed only by the nine copied test lines. Source files remained unchanged.

  • Inspected prefetched checks and failed-job logs: 123 passed, 5 failed, 1 skipped. FIRK CI passed. The AD tape error and GPU warning failure in PR CI exactly match master CI. SciMLSensitivity Core2/Core4 signatures in PR downstream CI match master downstream CI.

What I did not verify

  • The changed vector-tolerance branch: both revisions fail earlier during LinearSolve initialization.
  • Probe cases at 1e-6, which followed the aborting vector case, and the author’s tighter Van der Pol examples.
  • Local GPU, sensitivity AD, MPI, Reactant, package-wide, or heavy downstream suites.
  • The ModelingToolkit InterfaceI equation-orientation CI failure. Its signature is absent from the supplied master logs, so I have not classified it as pre-existing.

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)
Review: unreviewed
Session: 01a19733-7eb9-7c42-aae5-362416a735da

@ChrisRackauckas

Copy link
Copy Markdown
Member

🤖 Automated comment from an AI agent running as @ChrisRackauckas — not written or reviewed by Chris.

PR: #4934

Reviewed head: 78a3d07
Raw SHA-256: 29dca6935b9b1b27bbfff36288ad05aaa16ff913ce17392fe1eebed5c4030491
Source SHA-256: e6dacc18a5bb72901bcd6fd700a3cd37d63f86c314e8df8ec375c78a8e9b51f8
Source: standard-review-v1 (harness-log)
Reviewer session: 01a1d257-2c98-7812-a60e-e2868b7e7ac3
Slot: 1
Confidence: unknown
Own risk fixed before author rating/discussion: yes

Risk assessment

  • Risk: medium (M2)
  • Verdict: CHANGES
  • Reviewed head: 78a3d07

Review report

VERDICT: CHANGES
Round: 1
Head: 78a3d07
Slot: 1
CI: CI is not green (regress:SciMLSensitivity.jl/Core4/1 / Downstream Tests - Core4; ModelingToolkit.jl/InterfaceI/1 / Downstream Tests - InterfaceI)
Session: 01a1d257-2c98-7812-a60e-e2868b7e7ac3

Risk

medium (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).

  • Verdict: CHANGES
  • Round: 1
  • Blocking priorities: P2, P2
  • Deferred P2 issues: None supplied.

Blocking findings

  • [P2] Needs verification: affected numerical behavior was not exercised. lib/OrdinaryDiffEqFIRK/src/firk_perform_step.jl:160 and :163 change adaptive error scaling and Newton residual scaling. Package loading succeeded, but the FIRK Core suite, analytic accuracy probes, component-tolerance probes, and failing-before regression check never started. The runner waited over 70 minutes for the instructed load, memory, and Julia-worker limits, then was interrupted through its original execution session (exit 130). Run these checks before accepting the numerical change. This is a verification blocker, not a confirmed numerical defect.
  • [P2] The supplied downstream regression status remains unresolved. .github/workflows/Downstream.yml:23 and :25 select SciMLSensitivity Core4 and ModelingToolkit InterfaceI. The supplied CI status reports regressions in both. I did not inspect external CI logs or reproduce those downstream suites, so I cannot attribute the failures to this patch or establish that they also fail on the default branch. Resolve them or supply reproducible default-branch comparisons before merging.

Non-blocking findings

  • lib/OrdinaryDiffEqFIRK/test/ode_firk_tests.jl:285 — static coverage concern: the new assertion checks accepted-step counts without checking successful return codes or arrival at the final time, and its scalar tolerances exercise only one changed branch. Add completion assertions and a component-tolerance case. Existing adaptive cross-algorithm accuracy coverage at lines 332–343 is relevant; the analytic convergence tests at lines 180–186 use adaptive=false and bypass the changed initialization branch.

What I ran

AI 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.

  • git rev-parse HEAD confirmed the full head above; git diff origin/master...HEAD covered three files, 12 insertions and three deletions. Read the affected implementation, caches, interpolation setup, tests, and risk instructions. Root AGENTS.md and CLAUDE.md were absent.
  • git diff --check origin/master...HEAD exited 0. Final git status --porcelain=v1 was empty.
  • uv run --locked python --version printed Python 3.15.0 and warned that --locked has no effect outside a project. No uv.lock or Python project was found in the checkout.
  • julia --version printed julia version 1.13.2. No Julia Manifest was present. Constructed a scratch environment from FIRK's declared dependencies, test targets, compatibility bounds, and checkout source paths, preserving the supplied layered JULIA_DEPOT_PATH and requested TMPDIR.
  • julia --startup-file=no --project=<scratch-env> -e 'using Pkg; Pkg.instantiate(;allow_autoprecomp=false); Pkg.status()' exited 0. Resolved FIRK 2.8.10, Core 4.19.0, DiffEqBase 7.21.4, SciMLBase 3.58.0, and LinearSolve 5.18.3. This created a scratch Manifest, rather than reproducing a repository lockfile.
  • A separate julia --startup-file=no --project=<scratch-env> -e 'using OrdinaryDiffEqFIRK; println("Loaded FIRK from ", pathof(OrdinaryDiffEqFIRK)); println("Julia ", VERSION)' exited 0 and confirmed loading this checkout under Julia 1.13.2.
  • Started the scratch runner for targeted probes, GROUP=Core with lib/OrdinaryDiffEqFIRK/test/runtests.jl, and a parent-initializer negative control. Its log contains resource checks only. The Core and negative-control logs were never created. Confirmed that the waiting runner was gone after interruption; no started command remains running.

What I did not verify

No 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 u'=-u² against its analytic solution; and the new step-count assertion with the initializer restored from the PR parent in memory. Neither failing-before nor passing-after behavior is established.

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
Conversation: local session 01a1d257-2c98-7812-a60e-e2868b7e7ac3 (no session URL)

@singhharsh1708

Copy link
Copy Markdown
Member Author

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.

This branch has not been deployed

No deployments
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.

In-place RadauIIA3 uses reltol^(2/3) for its internal tolerance, out-of-place uses reltol^(3/4)

2 participants