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

SII based plot recipe now exposes internal, non-user variables #763

Open
isaacsas opened this issue Aug 6, 2024 · 8 comments
Open

SII based plot recipe now exposes internal, non-user variables #763

isaacsas opened this issue Aug 6, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@isaacsas
Copy link
Member

isaacsas commented Aug 6, 2024

Sometime after 9.10 the plot recipe has started to include propensity variables within plots, i.e. compare

https://docs.sciml.ai/JumpProcesses/v9.12/tutorials/discrete_stochastic_example/c355f569.svg

vs. the 9.10.1 version:

https://docs.sciml.ai/JumpProcesses/v9.10/tutorials/discrete_stochastic_example/86922c19.svg

I assume this is a plot-recipe change in SciMLBase, but don't know how / why it was ignoring them before.

@isaacsas isaacsas added the bug Something isn't working label Aug 6, 2024
@ChrisRackauckas
Copy link
Member

I didn't know it dropped some before. The current version is what we'd considered the correct one though?

@isaacsas
Copy link
Member Author

isaacsas commented Aug 7, 2024

I thought the goal was to hide the ExtendedJumpArray aspect from users and make solution objects look as much like ODE solutions as possible?

@ChrisRackauckas
Copy link
Member

oh true. Yeah I see, that's why they were hidden 😅. I'm not sure why that would have changed with a release. That's pretty hard to test too. I may have to dig in.

@isaacsas
Copy link
Member Author

isaacsas commented Aug 7, 2024

FWIW, the last version that didn't have this behavior within a release was on SciMLBase 2.10. We then jumped with 9.11. to SciMLBase 2.31 where it appears. For JumpProcesses the change occurred with 9.11's release, but on quick glance I see nothing in the merged PRs that could cause this within JumpProcesses.

@isaacsas
Copy link
Member Author

isaacsas commented Aug 7, 2024

OK, I've verified the issue appears with SciMLBase 2.12 which had only this PR:

#572

which updated the plot recipe to use SII. So it seems that changed the indexing in a way that the propensity values are now being plotted.

Here is a MWE if it would be useful for testing:

 using JumpProcesses, OrdinaryDiffEq, Plots
rate(u,p,t) = (1+t)*u[1]
affect(integrator) = (integrator.u[1] -= 1; nothing)
vrj = VariableRateJump(rate,affect)
oprob = ODEProblem((du,u,p,t) -> (du .=0; nothing), [10.0], (0.0, 10.0))
jprob = JumpProblem(oprob, Direct(), vrj)
sol = solve(jprob, Tsit5())
plot(sol)

Should I move this issue to SciMLBase?

@ChrisRackauckas
Copy link
Member

Yeah. The plot recipe does not have a real interface to opt variables in/out by default. It worked mostly because of a quirk of indexing and size checking that we probably shouldn't be relying on. It needs to be made into a real documented and tested interface.

@isaacsas isaacsas transferred this issue from SciML/JumpProcesses.jl Aug 8, 2024
@isaacsas
Copy link
Member Author

isaacsas commented Aug 8, 2024

I transferred this to SciMLBase, since my understanding is this requires changes in how the plot recipe works and isn't something we can handle at the level of JumpProcesses currently.

@isaacsas
Copy link
Member Author

isaacsas commented Aug 8, 2024

It does seem like it would be nice to have an interface to separate internal solver variables from user variables that could be used by both solution evaluation and plot recipes. Maybe it only impacts JumpProcesses via the ExtendedJumpArrays right now, but I'd imagine there are other use cases where solvers might want to internally augment user-variables with additional variables, but then not expose those back to users (or at least not expose them in default views -- having them available in a more advanced interface still makes sense).

@isaacsas isaacsas changed the title Plots now include rates for systems with VariableRateJumps SII based plot recipe now exposes internal, non-user variables Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants