Add EnsembleProblem syntax to example #142
Conversation
Codecov Report
@@ Coverage Diff @@
## master #142 +/- ##
=======================================
Coverage 88.56% 88.56%
=======================================
Files 23 23
Lines 1067 1067
=======================================
Hits 945 945
Misses 122 122 📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
|
|
||
| function prob_func(prob,i,repeat) | ||
| remake(prob, p=[Dnval[i],Dpval[i]]) |
There was a problem hiding this comment.
You should probably add an index check https://mtk.sciml.ai/dev/basics/FAQ/#Getting-the-index-for-a-symbol in case it's reordered.
There was a problem hiding this comment.
Without having dug around inside remake, is this something that could be handled internally there to prevent reordering? Seems dangerous to have exposure to invisible bugs like this that users wouldn't foresee unless they crossed this FAQ.
There was a problem hiding this comment.
@ChrisRackauckas thoughts on this? ^
There was a problem hiding this comment.
I'm not sure. Right now parameters never reorder, so technically this is correct. And with states, the reordering comes with reduction, so just passing an array of the right size will throw an error. But this is something we need to be working on. If an ODEProblem has a symbolic base, we should probably require a force=true or something to allow raw arrays to remake.
There was a problem hiding this comment.
I don't think we should have that. If one does not want reordering, then just don't use structural_simplify. Reordering is one of the most fundamental assumptions we have in MTK. Otherwise, there is not much point in giving states names other than for convenience.
There was a problem hiding this comment.
Well we'd need a force=true behavior internally anyways, but it would make sense to add a safety check and assume people will be using transformed systems.
There was a problem hiding this comment.
Seems to me that if you are defining your problem symbolically, and therefore are using named states and structural_simplify, then remake should require a Dict/Array{Pair} for p as in the system definition. This would also enhance consistency of the interface.
There was a problem hiding this comment.
remake should now work here with the pairs?
|
@thompsonmj can you update this to use symbolic remake? |
and close #104