Remove the hasmethod check

[KristofferC]

The hasmethod check in

DiffEqBase.jl/src/callbacks.jl

Line 622 in 01fd853

if hasmethod(reeval_internals_due_to_modification!,

has been reported to have quite severe runtime penalty, at least on 1.10.2.

As far as I understand, this is some kind of compatibility check so moving that to top-level would be preferable.

[oscardssmith]

I though as of JuliaLang/julia#48639, it should be done at compile time.

[KristofferC]

Yes, but it has been reported not to happen apparently.

[oscardssmith]

@vtjnash any idea why this would happen?

[vtjnash]

kwarg matching is a runtime feature, so it doesn't have a compile time component

[KristofferC]

@oscardssmith, in which SciMLBase version was this new way of calling reeval_internals_due_to_modification! introduced? We can just do a top-level check for that version instead.

[ChrisRackauckas]

https://github.com/SciML/SciMLBase.jl/releases/tag/v2.53.0 is the scimlbase part, but it's actually a downstream requirement on SciML/OrdinaryDiffEq.jl#2467 existing (and figuring out what OrdinaryDiffEqCore version that's a part of is a doozy due to the issues with the tooling 😅)

[oscardssmith]

can't we at this point just delete the hasmethod check since it only exists for compatibility with old versions of OrdinaryDiffEq?

[KristofferC]

I deleted all apply_discrete_callback! from DiffEqBase and tests still pass... Wat.

[KristofferC]

but it's actually a downstream requirement on SciML/OrdinaryDiffEq.jl#2467

Wow, that is very weird how another package extends a method with a new keyword like that.

[ChrisRackauckas]

Agreed, we really need JuliaLang/julia#55516. OrdinaryDiffEq and DiffEqBase are split mostly to lesson the load times of Sundials.jl and not for any real code separation reason.

[KristofferC]

It is still kind of weird how one function takes different kwargs depending on the integrator...

[ChrisRackauckas]

All of the integrators have been updated so this interface now just has kwargs for all current releases.

[KristofferC]

I was looking at https://github.com/SciML/SciMLBase.jl/blob/17f45482531981c25274dd9f0a7d8b934bc0a50b/src/integrator_interface.jl#L323-L335 which does not have a kwarg, or?

[ChrisRackauckas]

It's missing the kwarg. We need to update the interface spec, but since you can't really test interfaces well downstream that's a bug that just hadn't been caught yet. We need some language tooling to help here 😅.