Skip to content
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

fix Rosenbrock methods when eltype(u)!=eltype(tspan) #2051

Closed
wants to merge 17 commits into from

Conversation

oscardssmith
Copy link
Contributor

@oscardssmith oscardssmith commented Nov 3, 2023

MWE

using OrdinaryDiffEq
ode_f(du, u, p, t) = du[1] = -u[1]
prob = ODEProblem(ode_f, [1f0], (0.0,1.0))
solve(prob, Rodas5P())

@oscardssmith oscardssmith changed the title fix build_grad_config when eltype(u)!=eltype(tspan) fix build_grad_config when eltype(u)!=eltype(tspan) Nov 3, 2023
@oscardssmith oscardssmith changed the title fix build_grad_config when eltype(u)!=eltype(tspan) fix Rosenbrock methods when eltype(u)!=eltype(tspan) Nov 3, 2023
@ChrisRackauckas
Copy link
Member

Test failures look real.

@oscardssmith
Copy link
Contributor Author

@ChrisRackauckas the main real test failure I'm seeing is from https://github.com/oscardssmith/OrdinaryDiffEq.jl/blob/5ab14bf88d72f2b74ba1253aa6e04874e8a5e06c/test/interface/mass_matrix_tests.jl#L302 where we are testing solving an ODE with rosenbrock methods that has a zero mass matrix (but not looking at the results). Can I just change the mass matrix here, or is the test testing something reasonable?

@ChrisRackauckas
Copy link
Member

It's from SciML/DifferentialEquations.jl#757 as the comment says, so yes as long as it keeps the key property of testing non vector u0 it's fine.

@ChrisRackauckas
Copy link
Member

https://buildkite.com/julialang/ordinarydiffeq-dot-jl/builds/2906 looks like it's just throwing the wrong error now in that case, but is otherwise fine? Or maybe that test was just relying on the different types to be broken 😅 .

@oscardssmith
Copy link
Contributor Author

no. this is still slightly wrong. the remaining problem is that we are still making a minor mistake calculating the rate_prototype in the unitful case that is hard to fix without depending on unitful

@ChrisRackauckas
Copy link
Member

Is this done? Rebase?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants