Tighten the interval returned by InternalITP when there's an exact zero crossing

[DaniGlez]

Background is here: https://github.com/SciML/DiffEqBase.jl/pull/1127/files#r2022436123

To summarize: inside the branch where the callback condition is evaluated to an exact zero, this PR fills the left and right fields of the solution with the location of the zero crossing instead of the values at the start of that ITP iteration.

[DaniGlez]

BTW, in that branch we assume that yps is an exact zero because it's an else after if yps > T0 and elseif yps < T0, but a NaN would also fall into that branch; should an appropriate exception be raised in that case?

[oscardssmith]

I do think this code is missing a branch for if right and left get within 1 ULP of each other. I think currently for a function like f(x) = x^2-2 it will run to maxiters since the function never hits 0.

[DaniGlez]

Shouldn't that case be covered by the if later down in the function?

DiffEqBase.jl/src/internal_itp.jl

Line 84 in 0874ff4

if nextfloat_tdir(left, left, right) == right

[oscardssmith]

This looks good to me. Would you object to either you adding the change from https://github.com/SciML/DiffEqBase.jl/actions/runs/14193284895/job/39762654888?pr=1137 or vice versa? There are some CI failures here that look potentially related (on both branches) so it would be nice to consolidate the changes so I can only investigate once.

[DaniGlez]

Either is fine by me, whatever is most convenient for you

[DaniGlez]

I've incorporated the change from #1136 here for convenience

[oscardssmith]

Looks good to me! Let's see if CI agrees.

[DaniGlez]

CI not too happy, it seems

[DaniGlez]

In this CI run, the only test failure that I can connect to callback rootfinding with InternalITP is this one: https://github.com/SciML/DiffEqBase.jl/actions/runs/14203971489/job/39797246048?pr=1137#step:6:17566

LoadError: MethodError: no method matching exponent(::ReverseDiff.TrackedReal{Float64, Float64, Nothing})

[oscardssmith]

I think the CI results might be bogus let's try again. We really don't want to be autodiffing the ITP so hopefully we aren't...

[DaniGlez]

First one looks relevant: https://github.com/SciML/DiffEqBase.jl/actions/runs/14233499341/job/39888542399?pr=1137

[DaniGlez]

Yeah, I think I can reproduce it (from https://github.com/SciML/DelayDiffEq.jl/blob/master/test/integrators/events.jl#L8 ). It seems that the callback location is now shifted 1 ulp to the right compared to previous behaviour. So the test failure could be remediated by:

1. When building the solution, shift the left field 1 ulp to the left, which I think would restore pre-6.167.1 behavior
2. Accept the new behavior, since the crossing is an exact zero and arguably the behavior is more sensible, and as a consequence amend the relevant DelayDiffEq tests to replace the prevfloat(2.6) instances in that test set by a plain 2.6

[oscardssmith]

@ChrisRackauckas thoughts on the right approach here?

[ChrisRackauckas]

Yeah if we are now better at stopping at the exact zero, that's better. We should accept that and change the test. Our goal has always been to get there, it just wasn't robust before.

[oscardssmith]

sounds good. In that case, I think this is ready to merge and we can fix the test.