Switch find_first_continuous_callback to use a generated implementation.

[meson800]

Summary

As mentioned in SciML/DifferentialEquations.jl#971, the current recursive method for identifying the first continuous callback can cause the compiler to give up on type inference, especially when there are many callbacks. The fallback then allocates.

This switches this function to using a generated function (along with an inline function that takes splatted tuples). Because this generated function explicitly unrolls the tuple, there are no type inference problems.

Testing

I added a test that allocates using the old implementation (about 19kb allocations!) but does not with the new function. If I remove one of the callbacks, the allocations no longer happen with the recursive function, so it might be dependent on e.g. the specific Julia version to determine the point where the compiler gives up on type inference.

It passes all of the other DiffEqBase.jl, but I was wondering if there was a better test suite I could do to ensure this is working as expected. It seems to work in my ODE code, but I'm a little wary since I touched some pretty core code.

[ai-maintainer]

AI-Maintainer Review for PR - Switch find_first_continuous_callback to use a generated implementation

Title and Description 👍

The Title and description are clear, concise and helpful

The title and description of the pull request are clear and concise. They effectively communicate the purpose of the changes, which is to switch the `find_first_continuous_callback` function to use a generated implementation to avoid type inference problems and unnecessary allocations. The author also mentions the addition of a test to verify the correctness of the changes.

Scope of Changes 👍

The changes are narrowly focused

The changes in this pull request are narrowly focused on resolving a specific issue related to the `find_first_continuous_callback` function. The author is not trying to resolve multiple issues simultaneously, which is a good practice as it makes the changes easier to review and test.

Testing 👍

The author has tested the changes and provided a description of the testing process

The author has added a test to verify the changes and has described the testing process in the pull request description. The author also expresses a willingness to explore additional testing options if necessary, which shows a good understanding of the importance of thorough testing.

Documentation 👎

Some functions lack docstrings

The `find_first_continuous_callback` function lacks a docstring. It is recommended to add a docstring to this function to describe its behavior, arguments, and return values. This will make the code easier to understand for other developers.

Suggested Changes

• Please add a docstring to the find_first_continuous_callback function. This should include a brief description of what the function does, its arguments, and its return values. This will make the code easier to understand and maintain.

Potential Issues

• I didn't spot any potential issues in the code. The changes seem to be well thought out and the new implementation appears to be more efficient than the old one. However, it's always a good idea to keep an eye out for any unexpected behavior that might arise due to the changes.

Reviewed with AI Maintainer

[ChrisRackauckas]

the allocations no longer happen with the recursive function, so it might be dependent on e.g. the specific Julia version to determine the point where the compiler gives up on type inference.

yes, 16 is the compiler cutoff for tuple recursion.

It passes all of the other DiffEqBase.jl, but I was wondering if there was a better test suite I could do to ensure this is working as expected. It seems to work in my ODE code, but I'm a little wary since I touched some pretty core code.

The downstream OrdinaryDiffEq tests should be sufficient.

[ChrisRackauckas]

Looks good!