symbolic save idxs, new observed #392
Conversation
Codecov Report
@@ Coverage Diff @@
## master #392 +/- ##
===========================================
- Coverage 52.25% 39.22% -13.03%
===========================================
Files 46 46
Lines 3554 3602 +48
===========================================
- Hits 1857 1413 -444
- Misses 1697 2189 +492
... and 22 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Isn't this needed in every solution type for the indexing? |
|
is |
|
Are the test failures related? |
| i = get(A.sym_map, sym, nothing) | ||
| else | ||
| i = sym_to_index(sym, A) | ||
| end | ||
| elseif all(issymbollike, sym) | ||
| if has_sys(A.prob.f) && all(Base.Fix1(is_param_sym, A.prob.f.sys), sym) || |
There was a problem hiding this comment.
i isn't defined in this branch. Maybe this causes the test failure.
There was a problem hiding this comment.
that branch always returns
|
Remaining failures look unrelated |
| @@ -39,6 +39,7 @@ struct DAESolution{T, N, uType, duType, uType2, DType, tType, P, A, ID, DE} <: | |||
| interp::ID | |||
| dense::Bool | |||
| tslocation::Int | |||
| sym_map::MType | |||
There was a problem hiding this comment.
Can this be done using prob.f.sys? Requiring explicit maps isn't great.
There was a problem hiding this comment.
no, because save_idxs isn't known at sys construction stage, only at solve
There was a problem hiding this comment.
checking out the OrdinaryDiffEq part of this makes this all make a lot more sense to me.
src/problems/ode_problems.jl
Outdated
| kwargs...) where {iip} | ||
| _tspan = promote_tspan(tspan) | ||
| new{typeof(u0), typeof(_tspan), | ||
| isinplace(f), typeof(p), typeof(f), | ||
| typeof(kwargs), | ||
| typeof(problem_type)}(f, u0, _tspan, p, kwargs, problem_type) | ||
| typeof(problem_type)}(f, u0, _tspan, p, kwargs, problem_type, dense_output) |
There was a problem hiding this comment.
otherwise it breaks observed for odae and others
There was a problem hiding this comment.
Not ODAE sorry, but things like sol[x[1]^2+x[2]+norm(x)]. Since you don't know which states you won't have later at solve time it was impossible to get both to work so I needed a flag. I would need some kind of signal from problem construction time to post solve, to signal what to call the observed functions with
There was a problem hiding this comment.
What signal do you need other than save_idxs?
There was a problem hiding this comment.
we don't have save_idxs at problem construction time, only at solve time, look at the ODE PR
There was a problem hiding this comment.
@ChrisRackauckas thoughts on the above?
Merge order is:
Am trying to test each part seperately but a lot of this is very tightly coupled, I will test it composed in the MTK PR locally with all dev'd and notify when that passes