Skip to content

Cleanup event finding and change nudging condition#1260

Merged
ChrisRackauckas merged 4 commits intoSciML:masterfrom
dcourteville:refactor_detection
Feb 4, 2026
Merged

Cleanup event finding and change nudging condition#1260
ChrisRackauckas merged 4 commits intoSciML:masterfrom
dcourteville:refactor_detection

Conversation

@dcourteville
Copy link
Contributor

While implementing the change in nudging condition for #1245, I somehow ended up cleaning up all the event finding code to remove the redundant computations and useless branches, and factorize some code between ContinuousCallback and VectorContinuousCallback. If I didn't break anything it should make maintaining the code easier.
I know I shouldn't make two changes in one PR, I can split into two PR if its an issue.

@ChrisRackauckas
Copy link
Member

Draft?

@dcourteville dcourteville marked this pull request as ready for review January 20, 2026 10:19
@dcourteville
Copy link
Contributor Author

I'll update the doc for abstol in SciMLBase

@ChrisRackauckas
Copy link
Member

Null problem with callbacks: Test Failed at /home/runner/work/DiffEqBase.jl/DiffEqBase.jl/test/downstream/null_de.jl:165
  Expression: solve(prob_with_cb, Tsit5(); callback = cb)
    Expected: Exception
  No exception thrown

Stacktrace:
 [1] top-level scope
   @ ~/work/DiffEqBase.jl/DiffEqBase.jl/test/downstream/null_de.jl:151
 [2] macro expansion
   @ /opt/hostedtoolcache/julia/1.12.4/x64/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
 [3] macro expansion
   @ ~/work/DiffEqBase.jl/DiffEqBase.jl/test/downstream/null_de.jl:165 [inlined]
Null problem with callbacks: Test Failed at /home/runner/work/DiffEqBase.jl/DiffEqBase.jl/test/downstream/null_de.jl:181
  Expression: init(prob_with_cb, Tsit5(); callback = cb)
    Expected: Exception
  No exception thrown

update those, downstream now handles that so it just needs to be set to passing. we will need to see downstream tests because they have a lot of callback stuff

@dcourteville
Copy link
Contributor Author

Can this be merged or is there some other issues ?

@ChrisRackauckas ChrisRackauckas merged commit cb4efd0 into SciML:master Feb 4, 2026
42 of 46 checks passed
@ChrisRackauckas
Copy link
Member

This seemed to have some uncaught downstream consequences:

I can handle the GPU things, that's from using non-public API in DiffEqGPU. But the MWE @AayushSabharwal found seems more central as a repeat callback detection failure. I think because of how much stuff looks breaking right now, it might be good to revert, figure out the repeat callback, and then re-merge, and I'll get the GPU update setup and ready for that. @dcourteville can you look into that repeat callback issue?

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.

3 participants