fix: consider type of t0 in promote_u0
#1039
Conversation
src/forwarddiff.jl
Outdated
| eltype(u0) <: ForwardDiff.Dual && return u0 | ||
| T = anyeltypedual(p) | ||
| if T <: ForwardDiff.Dual | ||
| return T.(u0) | ||
| end | ||
| T = anyeltypedual(t0) | ||
| if T <: ForwardDiff.Dual | ||
| return T.(u0) |
There was a problem hiding this comment.
This isn't necessarily correct. If these things are 3 different duals we should error. It needs to try to promote to the common dual type or error.
There was a problem hiding this comment.
This apparently breaks tests, since in some cases it tests with two different duals. Previously, if u0 was a dual it would return u0 without checking if p is a dual or trying to promote. I added the same thing here to make tests pass. Should it be reverted, and the tests updated accordingly? And if so, wouldn't that be breaking?
AayushSabharwal
force-pushed
the
as/promote-u0-tspan
branch
3 times, most recently
from
75466ab
to
9c375c7
Compare
|
What's the state here? |
|
|
|
We just need to handle Measurement differently. Does this actually show up in the tests? |
I think the other way around, Measurement with dual numbers in it. Make a PR to Measurements.jl |
|
Turns out that's not possible because Measurements requires that the wrapped type be |
|
Ask Mose if that can be relaxed. |
|
It seems unlikely given JuliaPhysics/Measurements.jl#100 (comment). Also relaxing that would require breaking Measurements |
|
Okay then we should just change the test if that's fundamental |
|
I'm not entirely sure why that test has duals in tspan. It looks like it's meant to test that the rhs function is unwrapped when the unknown is a measurement, which is already tested. If it's meant to test that this also works when |
|
Yeah seems fundamental. I think it can be removed as it was only working because the expected conversion, i.e. differentiation of the solve, didn't actually take place. |
AayushSabharwal
force-pushed
the
as/promote-u0-tspan
branch
from
986a43d
to
114ad0b
Compare
Close SciML/ModelingToolkit.jl#2721
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here.