Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rate function interface #314

Open
isaacsas opened this issue Apr 12, 2023 · 4 comments
Open

rate function interface #314

isaacsas opened this issue Apr 12, 2023 · 4 comments

Comments

@isaacsas
Copy link
Member

isaacsas commented Apr 12, 2023

Is there a reason that rate functions for jumps don't take integrators? One disadvantage of this design is that one can't use the new symbolic indexing options in them.

@ChrisRackauckas
Copy link
Member

They do take integrators, but the issue is that it needs to be interpolated to the right point.

@isaacsas
Copy link
Member Author

isaacsas commented Apr 13, 2023

Ahh yes, I always mix it up with the jump rate definition. I guess I was actually thinking of the latter (which is of the form rate(u,p,t), i.e. doesn't take the integrator).

@isaacsas isaacsas transferred this issue from SciML/SciMLBase.jl Apr 13, 2023
@isaacsas isaacsas changed the title condition function interface rate function interface Apr 13, 2023
@isaacsas
Copy link
Member Author

@ChrisRackauckas what do you think about using SciMLBase.numargs to add support for single argument rate(integrator) functions? (Basically, we create a wrapper if passed a rate(u,p,t) function which forwards the appropriate integrator fields, letting us standardize on rate(integrator) as the real interface.)

My thought is this would ensure rate, condition, and affect functions across callbacks and jumps all have access to integrators. Users would then always have access to the symbolic indexing.

We could also then unify that rngs are stored in integrators across SciML, which fixes the issue that we store them in the JumpAggregations here, and means users would be able to use the internal rng we are using in their callback/jump functions. (It also avoids having to break/modify the definition of rate and affect functions to support an explicit rng argument.) Of course, it would then require adding this field across to most ODE/SDE integrators, which I guess could be a lot of work?

@isaacsas
Copy link
Member Author

Ahh, I see that this won't work for variable rates since they have to support being called without integrators via the ODE derivative interface:

update_jumps!(du, u, p, t, length(u.u), jumps...)

That's unfortunate as this seemed like a clean way to pass around rngs too.

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

No branches or pull requests

2 participants