Prevent rationalization in QNDF

[hersle]

This fixes #2921 because γₖ now remain rational, and are not converted to floats due to the tTypeNoUnits conversion.

I don't know whether this is ok. Why do only BDF/NDF methods do this tTypeNoUnits conversion when constructing their cache? Can I also remove all the other tTypeNoUnits conversions in the same file?

[ChrisRackauckas]

Why would this be faster? Multiplication by a rational number requires doing a division, which is slower than just a floating point multiplication. All rationals should just be turned into floats for the actual calculation

[hersle]

The core of the problem is

OrdinaryDiffEq.jl/lib/OrdinaryDiffEqBDF/src/bdf_perform_step.jl

Line 906 in 9e60800

nlsolver.γ = β₀

Here nlsolver.γ is a rational, but β₀ is a float. This triggers the expensive conversion and rationalize call in #2921.

So we should either turn nlsolver.γ into a float, or β₀ into a rational. This PR currently does the latter. What do think is best?

[hersle]

It is faster:

using OrdinaryDiffEqBDF, ProfileCanvas, BenchmarkTools
f(du, u, p, t) = du .= 0.0
prob = ODEProblem(f, ones(10), (0.0, 1.0))
@btime sol = solve(prob, QNDF(); adaptive = false, dt = 1e-5, save_everystep = false)

  391.329 ms (100110 allocations: 4.59 MiB) # before this PR
  277.692 ms (100110 allocations: 4.59 MiB) # after this PR

But other solutions avoiding the rationalize would probably be as fast.

[oscardssmith]

we probably should make nlsolver.γ a float (presumably of the same type as our tolerances)

[ChrisRackauckas]

Yes the answer is to make nlsolver.γ match the tTypeNoUnits. Ultimately with it will always need to convert to that value in the end, so what you're doing now is just keeping it rational for just a little bit longer. I don't see that as numerically more accurate in any realistic sense with time being floating point, and if time is rational numbers then making it tTypeNoUnits will make the whole thing rational which is what you'd want in that case.

[hersle]

Makes sense. Does it make sense to do it only for NDF in this PR, or would you only want to do it for all solvers at once?

[ChrisRackauckas]

all at once.

[hersle]

Understandable, closing this for now as I don't have the capacity to go at that now.

[ChrisRackauckas]

I assume it would be pretty quick from what you have? I'll see if claude can just finish it.

[hersle]

Ok, thanks. I thought you meant this has to be gone through for every single solver in all of OrdinaryDiffEq? Or did you mean just the BDF family ones?

[ChrisRackauckas-Claude]

This has been addressed in #2923 which converts nlsolver.γ to tTypeNoUnits in build_nlsolver, following the approach suggested in the discussion (making nlsolver.γ match tTypeNoUnits for all solvers).

[hersle]

Perfect.