Skip to content

Conversation

@ChrisRackauckas-Claude
Copy link
Contributor

Summary

This PR fixes #2921 by converting the nlsolver.γ parameter to tTypeNoUnits in build_nlsolver, preventing expensive rationalize() calls during QNDF integration.

Problem

When using QNDF solvers, the nlsolver.γ was initialized as Rational{Int64} (e.g., Int64(1)//1). Later, during the step function, a Float64 value would be assigned to nlsolver.γ = β₀. This triggered Julia's conversion from Float64 to Rational{Int64}, which calls rationalize() - an expensive operation that was consuming significant computation time.

Solution

Convert γ to tTypeNoUnits (typically Float64) in build_nlsolver before:

  1. Using it in nlp_params tuples for NonlinearSolveAlg
  2. Constructing the NLSolver object

This ensures the type parameter gamType is Float64 from the start, so later assignments are just Float64 to Float64 (no conversion needed).

Changes

  • lib/OrdinaryDiffEqNonlinearSolve/src/utils.jl: Added γ = tTypeNoUnits(γ) conversion in both Val{true} (in-place) and Val{false} (out-of-place) versions of build_nlsolver

Benchmark Results

using OrdinaryDiffEqBDF, BenchmarkTools
f(du, u, p, t) = du .= 0.0
prob = ODEProblem(f, ones(10), (0.0, 1.0))
@btime solve(prob, QNDF(); adaptive = false, dt = 1e-5, save_everystep = false)
  • Before: ~391 ms
  • After: ~264 ms (~32% speedup)

Test Plan

  • Pkg.test("OrdinaryDiffEqBDF") passes (70 tests)
  • Pkg.test("OrdinaryDiffEqNonlinearSolve") passes
  • Pkg.test("OrdinaryDiffEqSDIRK") passes
  • Manual verification that QNDF, QNDF1, QNDF2, and FBDF solvers work correctly

Closes #2921

🤖 Generated with Claude Code

This fixes SciML#2921 by ensuring the nlsolver γ parameter is converted to
tTypeNoUnits (typically Float64) in build_nlsolver. Previously, γ was
passed as a Rational{Int64} which caused the NLSolver's gamType to be
Rational{Int64}. When later assigning a Float64 value to nlsolver.γ
(e.g., β₀ in QNDF), Julia would call rationalize() to convert the float
to rational, which is very slow.

The fix converts γ to tTypeNoUnits in both the in-place (Val{true}) and
out-of-place (Val{false}) versions of build_nlsolver:
1. When using NonlinearSolveAlg: convert γ before it's used in nlp_params
2. At NLSolver construction: always pass tTypeNoUnits(γ)

Benchmark improvement for QNDF:
- Before: ~391ms
- After:  ~264ms (>30% speedup)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@ChrisRackauckas ChrisRackauckas merged commit b80bd3b into SciML:master Dec 8, 2025
134 of 247 checks passed
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.

QNDF slowed down while rationalizing floats

2 participants