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

Sundials 4.12 and newer break KiteModels #392

Closed
ufechner7 opened this issue Feb 18, 2023 · 4 comments
Closed

Sundials 4.12 and newer break KiteModels #392

ufechner7 opened this issue Feb 18, 2023 · 4 comments

Comments

@ufechner7
Copy link

ufechner7 commented Feb 18, 2023

My package https://github.com/ufechner7/KiteModels.jl works fine with Sundials 4.11.4 and older, but 4.12.0 and newer make it fail. In addition pre-compilation of Sundials 4.11.4 only works with DiffEqBase = "<6.116.0".

The following unit test shows the problem:

# unittest for Sundials.jl, works fine with 4.11.4 and older
using StaticArrays, Sundials, Test

const KVec3    = MVector{3, Float64}

Base.@kwdef mutable struct KPS4{S, T, P}
    v_apparent::T =       zeros(S, 3)
end

const kps4 = KPS4{Float64, KVec3, 6+4+1}()

function residual!(res, yd, y::MVector{S, Float64}, s::KPS4, time) where S
end

function init_sim!(t_end=1.0)
    y0 = MVector{62, Float64}([13.970413450119487, 0.0, 21.238692070636343, 27.65581376097752, 0.0, 42.66213714321849, 40.976226230518435, 0.0, 64.314401166278, 53.87184032029182, 0.0, 86.22231803750196, 66.28915240374937, 0.0, 108.4048292516046, 78.17713830204762, 0.0, 130.87545423106485, 79.56930502428155, 0.0, 135.70836376062155, 80.90383289255747, 0.0, 137.7696816741141, 80.60126812407692, 2.4016533873456325, 135.3023287520457, 80.60126812407692, -2.4016533873456325, 135.3023287520457, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 150.0, 0.0])
    yd0= MVector{62, Float64}([0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, -9.81, 0.0, 0.0, -9.81, 0.0, 0.0, -9.81, 0.0, 0.0, -9.81, 0.0, 0.0, -9.81, 0.0, 0.0, -9.81, 0.0, 0.0, -9.81, 0.0, 0.0, -9.81, 0.0, 0.0, -9.81, 0.0, 0.0, -9.81, 0.0, 0.0])
    differential_vars = ones(Bool, length(y0))
    solver  = IDA(linear_solver=Symbol("GMRES"), max_order = 4)
    tspan   = (0.0, t_end) 
    abstol  = 0.0006 # max error in m/s and m
    prob    = DAEProblem(residual!, yd0, y0, tspan, kps4, differential_vars=differential_vars)
    integrator = Sundials.init(prob, solver, abstol=abstol, reltol=0.001)
end

@testset "test_simulate        " begin
    STEPS = 500
    integrator = init_sim!()
end
nothing
@ViralBShah
Copy link
Contributor

Is this still an issue?

@ChrisRackauckas
Copy link
Member

I'm not sure we ever expected MArray initial conditions to work with Sundials. It's kind of interesting that it did work at one point through auto-conversions, but since Sundials needs an array pointer to work internally on the values there were just implicit conversions and so it wasn't any faster. Now that we've constrained the package to more clearly require Array{Float64}, this "broke", but is really making it front-and-center that it always required Array{Float64}. To get the equivalent behavior as before, one would need to convert in the f function which is fine. I understand it was quite convenient that it worked before, but I think exposing the solver's true usage makes the performance aspect a lot more clear and so I'll say it was an accident this ever worked without being explicit, and it's not a bug that it's now an error.

@ufechner7
Copy link
Author

What I don't like is that you say that you closed this as completed, because that is not true. You closed it as "Won't fix". Is this a limitation of Github that you cannot close a bug as "Won't fix"?

@ChrisRackauckas ChrisRackauckas closed this as not planned Won't fix, can't repro, duplicate, stale Jul 15, 2023
@ChrisRackauckas
Copy link
Member

Just an oversight.

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

No branches or pull requests

3 participants