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.

Sign up for GitHub

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

Jump to bottom

Segfault from bad @inbounds in interpolation when solve fails #458

Closed
Keno opened this issue Jun 17, 2023 · 4 comments · Fixed by #476
Closed

Segfault from bad @inbounds in interpolation when solve fails #458

Keno opened this issue Jun 17, 2023 · 4 comments · Fixed by #476
Assignees
@oscardssmith

Comments

@Keno
Copy link
Contributor

Keno commented Jun 17, 2023

I'm seeing a segfault trying to index into a solution that failed during solve. Turning on boundschecks, we see the following:

[IDAS ERROR]  IDASolve
  At t = 0 and h = 1.49012e-18, the error test failed repeatedly or with |h| = hmin.

Error During Test at /home/keno/.julia/dev/CedarSim/test/inverter.jl:58
  Test threw exception
  Expression: isapprox_deftol(sol(0.0, idxs = sys.node_d), 0.0)
  BoundsError: attempt to access 1-element Vector{Float64} at index [2]
  Stacktrace:
   [1] getindex(A::Vector{Float64}, i1::Int64)
     @ Base ./essentials.jl:13 [inlined]
   [2] interpolation(tvals::Vector{Float64}, id::SciMLBase.HermiteInterpolation{Vector{Float64}, Vector{Vector{Float64}}, Vector{Vector{Float64}}}, idxs::Nothing, deriv::Type{Val{0}}, p::CedarSim.DefaultSim{Main.inverter_tests.var"#25#26"}, continuity::Symbol)
     @ SciMLBase ~/.julia/packages/SciMLBase/KcGs1/src/interpolation.jl:104
   [3] (::SciMLBase.HermiteInterpolation{Vector{Float64}, Vector{Vector{Float64}}, Vector{Vector{Float64}}})(tvals::Vector{Float64}, idxs::Nothing, deriv::Type, p::CedarSim.DefaultSim{Main.inverter_tests.var"#25#26"}, continuity::Symbol)
     @ SciMLBase ~/.julia/packages/SciMLBase/KcGs1/src/interpolation.jl:55
   [4] (::SciMLBase.DAESolution{Float64, 2, Vector{Vector{Float64}}, Nothing, Nothing, Nothing, Vector{Float64}, SciMLBase.DAEProblem{Vector{Float64}, Vector{Float64}, Tuple{Float64, Float64}, true, CedarSim.DefaultSim{Main.inverter_tests.var"#25#26"}, SciMLBase.DAEFunction{true, SciMLBase.FullSpecialize, Core.OpaqueClosure{Tuple{Vector{Float64}, Vector{Float64}, Vector{Float64}, CedarSim.DefaultSim{Main.inverter_tests.var"#25#26"}, Float64}, Nothing}, Nothing, Nothing, DAECompiler.var"#116#117"{Tuple{typeof(DAECompiler.zero_first!), Core.OpaqueClosure{Tuple{AbstractMatrix{Float64}, Vector{Float64}, Vector{Float64}, CedarSim.DefaultSim{Main.inverter_tests.var"#25#26"}, Float64, Float64}, Nothing}}}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, DAECompiler.DAEReconstructedObserved, Nothing, DAECompiler.TransformedIRODESystem}, @Kwargs{callback::Nothing, tstops::Vector{Float64}, initializealg::CedarSim.CedarDCOp{SciMLNLSolve.NLSolveJL{LineSearches.Static, SciMLNLSolve.var"#2#4"}}}, Vector{Bool}}, Sundials.IDA{:Dense, Nothing, Nothing}, SciMLBase.HermiteInterpolation{Vector{Float64}, Vector{Vector{Float64}}, Vector{Vector{Float64}}}, DiffEqBase.Stats})(t::Float64, ::Type{Val{0}}, idxs::DAECompiler.ScopeRef{DAECompiler.IRODESystem}, continuity::Symbol)
     @ SciMLBase ~/.julia/packages/SciMLBase/KcGs1/src/solutions/ode_solutions.jl:133
   [5] (::SciMLBase.DAESolution{Float64, 2, Vector{Vector{Float64}}, Nothing, Nothing, Nothing, Vector{Float64}, SciMLBase.DAEProblem{Vector{Float64}, Vector{Float64}, Tuple{Float64, Float64}, true, CedarSim.DefaultSim{Main.inverter_tests.var"#25#26"}, SciMLBase.DAEFunction{true, SciMLBase.FullSpecialize, Core.OpaqueClosure{Tuple{Vector{Float64}, Vector{Float64}, Vector{Float64}, CedarSim.DefaultSim{Main.inverter_tests.var"#25#26"}, Float64}, Nothing}, Nothing, Nothing, DAECompiler.var"#116#117"{Tuple{typeof(DAECompiler.zero_first!), Core.OpaqueClosure{Tuple{AbstractMatrix{Float64}, Vector{Float64}, Vector{Float64}, CedarSim.DefaultSim{Main.inverter_tests.var"#25#26"}, Float64, Float64}, Nothing}}}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, DAECompiler.DAEReconstructedObserved, Nothing, DAECompiler.TransformedIRODESystem}, @Kwargs{callback::Nothing, tstops::Vector{Float64}, initializealg::CedarSim.CedarDCOp{SciMLNLSolve.NLSolveJL{LineSearches.Static, SciMLNLSolve.var"#2#4"}}}, Vector{Bool}}, Sundials.IDA{:Dense, Nothing, Nothing}, SciMLBase.HermiteInterpolation{Vector{Float64}, Vector{Vector{Float64}}, Vector{Vector{Float64}}}, DiffEqBase.Stats})(t::Float64, ::Type{Val{0}}; idxs::DAECompiler.ScopeRef{DAECompiler.IRODESystem}, continuity::Symbol)
     @ SciMLBase ~/.julia/packages/SciMLBase/KcGs1/src/solutions/ode_solutions.jl:66 [inlined]
@oscardssmith oscardssmith self-assigned this Jul 17, 2023
@ChrisRackauckas
Copy link
Member

@oscardssmith were you going to look at this one? I think removing some @inbounds now that it's not needed in most scenarios could be helpful stuff anyways.

@oscardssmith
Copy link
Contributor

will do.

@oscardssmith oscardssmith mentioned this issue Aug 9, 2023
Merged
@oscardssmith
Copy link
Contributor

fixed by #476

@oscardssmith oscardssmith mentioned this issue Aug 10, 2023
Merged
@oscardssmith
Copy link
Contributor

Also the mwe is pretty simple:

f(out, du, u, p, t) = out .= [t]
prob = DAEProblem(f, [1.0], [1.0], (0, 2); differential_vars = [false])
sol = solve(prob, IDA())
sol(1.0)

sjdaines added a commit to sjdaines/Sundials.jl that referenced this issue Sep 23, 2023
@sjdaines
Fix handling of small timesteps
89a643d 
Sundials solver is allowed to warn and continue if it returns a timestep
with no change in t.
If the solver doesn't recover, it will fail with eg SciMLBase.ReturnCode.MaxIters

Reverts PR SciML#416, fixes
SciML#420

For CVode and ARKode, it looks like this is intended behaviour: the
Sundial solver emits a warning message, with an API call
    CVodeSetMaxHnilWarns
    ARKStepSetMaxHnilWarns
to set the maximum number of warning messages printed, which is exposed
to Julia as
    max_hnil_warns
see Sundials driver code https://github.com/LLNL/sundials/blob/e8a3e67e3883bc316c48bc534ee08319a5e8c620/src/cvode/cvode.c#L1339-L1347

For IDA, there is no API like this so it's not so clear what should happen,
however the behaviour prior to SciML#416 (restored by this PR)
for the f_noconverge test added in test/common_interface/ida.jl is to return
  SciMLBase.ReturnCode.MaxIters
which seems reasonable?

(NB: @oscardssmith I can't reproduce the issue implied by the title of
SciML#416 so perhaps what is missing here
is the original failure case that demonstrates the issue?
The call to solve in the f_noconverge test test/common_interface/ida.jl, returns SciMLBase.ReturnCode.MaxIters,
it doesn't return with no error?

Also SciML/SciMLBase.jl#458 shows IDA printing
  [IDAS ERROR]  IDASolve
    At t = 0 and h = 1.49012e-18, the error test failed repeatedly or with |h| = hmin.
which suggests that IDA did flag an error in that case ?
)
@sjdaines sjdaines mentioned this issue Sep 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oscardssmith oscardssmith

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix segfault when interpolating on failed solution oscardssmith/SciMLBase.jl
3 participants
@Keno @ChrisRackauckas @oscardssmith