-
Notifications
You must be signed in to change notification settings - Fork 6
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
Symbolic Jacobian sparsity #1606
Conversation
I did some digging. I found out these things:
|
There seems to be a problem with |
One thing to keep in mind here is that some basin dependencies are temporary and not 'active' at initialization. To get a proper sparsity pattern, all nodes should be active |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments. Let's have a look at the impact performance and binary size.
Calculating the Jacobian seems fast, but runtime performance went down a lot. The |
We should also have a look at https://github.com/adrhill/SparseConnectivityTracer.jl as an alternative, lighter dependency. |
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! You'll have to approve yourself ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a follow-up of #1606. Hopefully fixes the runtime of the HWS model.
This doesn't work yet, but I quickly tried this out yesterday, thought I might as well put it up to look if we should pursue this.
I think the main advantage is that we don't have to maintain our own code for determining the sparsity, which can more easily get out of sync. It does add some dependencies.
The error I get is with DataInterpolations not supporting
::SymbolicUtils.BasicSymbolic{Real}
, which looks like this closed issue: SciML/DataInterpolations.jl#168. DataInterpolations has an extension on Symbolics for this purpose, so perhaps we just need to create a minimal example and file an issue.It could be that there are a bunch more issues of this kind once this is resolved.
If it starts to work but we don't want it, we should perhaps use it to test our manual Jacobian sparsity.