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

Add BGC to ODE wrapper function #78

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add BGC to ODE wrapper function #78

wants to merge 8 commits into from

Conversation

radka-j
Copy link
Contributor

@radka-j radka-j commented Nov 8, 2024

This PR adds a bgc_to_ode wrapper function which can be used to turn a BGC model into an ODEProblem object. The differential equations and inference examples have been simplified accordingly. I have run both examples to check the outputs remain the same and I have also added a small unit test.

NOTES: there were issues with compiling a dependancy of DifferentialEquations.jl (BoundaryValueDiffEq.jl) and in the process of debugging that it occurred to me that as long as we aim to only solve ODEs we should use the more lightweight package OrdinaryDiffEq.jl (which also does not have this dependancy). So I updated our dependencies accordingly.

@radka-j radka-j changed the base branch from refactor_light to main November 12, 2024 14:06
@radka-j radka-j marked this pull request as ready for review November 13, 2024 14:05
@radka-j radka-j requested a review from nanophyto November 13, 2024 14:09
@nanophyto
Copy link
Contributor

NOTES: there were issues with compiling a dependancy of DifferentialEquations.jl (BoundaryValueDiffEq.jl) and in the process of debugging that it occurred to me that as long as we aim to only solve ODEs we should use the more lightweight package OrdinaryDiffEq.jl (which also does not have this dependancy). So I updated our dependencies accordingly.

For SODEs we would also need StochasticDiffEq.jl. However, looking at the dependencies, this package also does not rely on BoundaryValueDiffEq.jl

Of course SODE's are a different beast and will require a different PR/more discussion, so sticking to OrdinaryDiffEq.jl for now makes sense to me.

@radka-j
Copy link
Contributor Author

radka-j commented Nov 13, 2024

Exactly, I think we should add dependencies as we need them.

@radka-j
Copy link
Contributor Author

radka-j commented Nov 14, 2024

The current set up will not work if the parameters are vectors or matrices. In those cases - what are the parameters that one is actually likely to want to do inference on? Is it all of them or is there an obvious subset to target?

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

Successfully merging this pull request may close these issues.

2 participants