Indexing fixes #408
Indexing fixes #408
Conversation
Co-authored-by: Aayush Sabharwal <aayush.sabharwal@gmail.com>
|
@AayushSabharwal can you check I've got the correct dispatches now for mutating parameters within JumpProblems ensuring |
|
If this looks ok I'll add some more symbolic tests for integrators and for remake. |
Pull Request Test Coverage Report for Build 8363146682Details
💛 - Coveralls |
| function SII.set_parameter!(prob::JumpProblem, val, idx) | ||
| ans = SII.set_parameter!(SII.parameter_values(prob), val, idx) | ||
|
|
||
| if using_params(prob.massaction_jump) | ||
| update_parameters!(prob.massaction_jump, prob.prob.p) | ||
| end | ||
|
|
||
| ans |
There was a problem hiding this comment.
While this is technically correct, set_parameter! will be called once for each parameter which would make this very expensive. I'll PR to SII to add a callback that runs at the end of setp (finalize_parameters_hook!(prob, ps)?)
There was a problem hiding this comment.
There was a problem hiding this comment.
Sounds good. I guess we could also use that for calling reset_aggregated_jumps! when integrators get updated in callbacks, which means one less thing for users to have to handle.
There was a problem hiding this comment.
@AayushSabharwal please ping me when a SII release with that callback is available so I can get this finished -- we need this for making updates to Catalyst for MTK9. Thanks!
There was a problem hiding this comment.
It's been released now @isaacsas
There was a problem hiding this comment.
Oops, I thought it was merged and would be available. Sorry 😅
|
@ChrisRackauckas this hadn't yet been updated for SciML/SymbolicIndexingInterface.jl#58, which is the interface we are supposed to use (and was only merged/released today). I'll open another PR to update this to the correct interface, hopefully later today or tomorrow. |
No description provided.