fix: remove implicit MTK dependency, other bug fixes, MTKv9 compatibility #603
Conversation
AayushSabharwal
force-pushed
the
as/mtk-v9
branch
from
0dbf958 to
1591f3e
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #603 +/- ##
===========================================
- Coverage 39.96% 26.35% -13.62%
===========================================
Files 54 54
Lines 4076 4113 +37
===========================================
- Hits 1629 1084 -545
- Misses 2447 3029 +582 ☔ View full report in Codecov by Sentry. |
AayushSabharwal
force-pushed
the
as/mtk-v9
branch
2 times, most recently
from
4683adc to
defdcb2
Compare
AayushSabharwal
force-pushed
the
as/mtk-v9
branch
from
defdcb2 to
20550cf
Compare
src/remake.jl
Outdated
| @@ -69,12 +69,9 @@ function remake(prob::ODEProblem; f = missing, | |||
| u0 = prob.u0 | |||
| end | |||
| if (eltype(p) <: Pair && !isempty(p)) || (eltype(u0) <: Pair && !isempty(u0)) # one is a non-empty symbolic map | |||
| hasproperty(prob.f, :sys) && hasfield(typeof(prob.f.sys), :ps) || | |||
| has_sys(prob.f) || | |||
There was a problem hiding this comment.
Let's solve SciML/DifferentialEquations.jl#922 at the same time here. If it doesn't have a system, then we shouldn't care what p is an just use it. p in a general diffeq is allowed to be a dictionary or Pair thing if people want, we shouldn't care.
But also, let's add a keyword argument interpret_symbolicmap = true and change the error message saying that this can be bypassed by setting that to false.
There was a problem hiding this comment.
Fixed in c503cf8
src/scimlfunctions.jl
Outdated
| function sys_or_symbolcache(sys, syms, paramsyms, indepsym = nothing) | ||
| if sys === nothing && (syms !== nothing || paramsyms !== nothing || indepsym !== nothing) | ||
| sys = SymbolCache(syms, paramsyms, indepsym) | ||
| end | ||
| return sys | ||
| end |
There was a problem hiding this comment.
The syms stuff is deprecated, I don't get why this is still staying around. If anything it should throw a big warning not to use it.
There was a problem hiding this comment.
Should I remove the keywords from the constructors or just @warn?
There was a problem hiding this comment.
I guess @warn since it was accidentally documented, and remove it from the docstrings.
There was a problem hiding this comment.
Fixed in 7be3e98
AayushSabharwal
force-pushed
the
as/mtk-v9
branch
from
1f88d7f to
7be3e98
Compare
Contrary to the name of the branch, this PR should be able to be merged without MTKv9
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here.