use SII.finalize_parameters_hook! when updating problems #409
Conversation
isaacsas
changed the title
src/problem.jl
Outdated
| function SII.set_parameter!(prob::JumpProblem, val, idx) | ||
| ans = SII.set_parameter!(SII.parameter_values(prob), val, idx) | ||
|
|
||
| function SII.finalize_parameters_hook!(prob::JumpProblem, p) | ||
| if using_params(prob.massaction_jump) | ||
| update_parameters!(prob.massaction_jump, prob.prob.p) |
There was a problem hiding this comment.
@AayushSabharwal is using prob.prob.p correct here? The argument p is just the newly changed parameters right? (And prob.prob.p should already be updated when this is called?)
There was a problem hiding this comment.
Yes. p is the parameter symbol(s) updated by setp. For the parameter values you'd need parameter_values(prob) (or prob.prob.p in this case). If I'm being picky I'd suggest using parameter_values(prob) or parameter_values(prob.prob) at least
There was a problem hiding this comment.
OK, I switched to parameter_values(prob).
Pull Request Test Coverage Report for Build 8440125933Details
💛 - Coveralls |
isaacsas
changed the title
|
If tests pass I'll plan to merge and make a new release. |
|
Awesome, good to see this is finally done. So then, Catalyst is close? Sorry for all of the chaos but I think we're getting to stable again. |
|
We're getting there! On the jump side all that is left after this is updating the parameter mapper for JumpSystems in MTK to work with SII correctly (i.e. the stuff that handles recalculating mass action rates when parameters change from the symbolic rate constant expressions). I'll then add some more comprehensive tests in MTK to go with that and make sure symbolic indexing works in the various desired ways with jump systems. I think aside from that @TorkelE has successfully run most Catalyst stuff now with the merged and open PRs we are finalizing. He has really put in a heroic effort updating stuff and patiently addressing my many comments, including a pretty through update of our tests. We are tracking what needs to be done for a MTK9 compatible release in SciML/Catalyst.jl#775 -- we have a bunch of other changes that will go in via other open PRs, but should hopefully get a release in the not too distant future (like getting 1:1 equivalence between the DSL and symbolic interface finally). |
Finishes #408