SciML / BoundaryValueDiffEq.jl

Boundary value problem (BVP) solvers for scientific machine learning (SciML)
Other
44 stars 33 forks source link

Error afer updating to BoundaryValueDiffEq v5: BoundsError in `interpolation` #118

Closed mateuszbaran closed 1 year ago

mateuszbaran commented 1 year ago

After updating to BoundaryValueDiffEq v5 CI shows the following error:

Torus in ℝ³: Error During Test at /home/runner/work/Manifolds.jl/Manifolds.jl/test/manifolds/embedded_torus.jl:93
  Test threw exception
  Expression: (sol_log(0.0))[1:2] ≈ p0x
  BoundsError: attempt to access 81-element Vector{Float64} at index [0]
  Stacktrace:
    [1] getindex
      @ ./essentials.jl:13 [inlined]
    [2] interpolation(tval::Float64, id::BoundaryValueDiffEq.MIRKInterpolation{Vector{Float64}, Vector{Vector{Float64}}}, idxs::Nothing, deriv::Type{Val{0}}, p::Tuple{Manifolds.EmbeddedTorus{Int64}, Manifolds.DefaultTorusAtlas, Tuple{Int64, Int64}}, continuity::Symbol)
      @ BoundaryValueDiffEq ~/.julia/packages/BoundaryValueDiffEq/fSVnX/src/interpolation.jl:81
    [3] (::BoundaryValueDiffEq.MIRKInterpolation{Vector{Float64}, Vector{Vector{Float64}}})(tvals::Float64, idxs::Nothing, deriv::Type, p::Tuple{Manifolds.EmbeddedTorus{Int64}, Manifolds.DefaultTorusAtlas, Tuple{Int64, Int64}}, continuity::Symbol)
      @ BoundaryValueDiffEq ~/.julia/packages/BoundaryValueDiffEq/fSVnX/src/interpolation.jl:8
    [4] AbstractODESolution
      @ ~/.julia/packages/SciMLBase/McEqc/src/solutions/ode_solutions.jl:75 [inlined]
    [5] #_#433
      @ ~/.julia/packages/SciMLBase/McEqc/src/solutions/ode_solutions.jl:66 [inlined]
    [6] AbstractODESolution
      @ ~/.julia/packages/SciMLBase/McEqc/src/solutions/ode_solutions.jl:64 [inlined]
    [7] (::ODESolution{Float64, 2, Vector{Vector{Float64}}, Nothing, Nothing, Vector{Float64}, Nothing, BVProblem{Vector{Float64}, Tuple{Float64, Float64}, true, Tuple{Manifolds.EmbeddedTorus{Int64}, Manifolds.DefaultTorusAtlas, Tuple{Int64, Int64}}, BVPFunction{true, SciMLBase.FullSpecialize, false, typeof(ManifoldsBoundaryValueDiffEqExt.chart_log_problem!), ManifoldsBoundaryValueDiffEqExt.var"#bc1!#3"{Vector{Float64}, Vector{Float64}}, UniformScaling{Bool}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, typeof(SciMLBase.DEFAULT_OBSERVED), Nothing, Nothing, Nothing}, SciMLBase.StandardBVProblem, Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}}, MIRK4{NewtonRaphson{nothing, AutoForwardDiff{0, Bool}, Nothing, typeof(NonlinearSolve.DEFAULT_PRECS), LineSearch{Static, AutoFiniteDiff{Val{:forward}, Val{:forward}, Val{:hcentral}}, Bool}}, MIRKJacobianComputationAlgorithm{AutoForwardDiff{nothing, Nothing}, AutoSparseForwardDiff{nothing, Nothing}, AutoSparseForwardDiff{nothing, Nothing}}}, BoundaryValueDiffEq.MIRKInterpolation{Vector{Float64}, Vector{Vector{Float64}}}, Nothing, Nothing})(t::Float64)
      @ SciMLBase ~/.julia/packages/SciMLBase/McEqc/src/solutions/ode_solutions.jl:64
    [8] macro expansion
      @ /opt/hostedtoolcache/julia/1.9.3/x64/share/julia/stdlib/v1.9/Test/src/Test.jl:478 [inlined]
    [9] macro expansion
      @ ~/work/Manifolds.jl/Manifolds.jl/test/manifolds/embedded_torus.jl:93 [inlined]
   [10] macro expansion
      @ /opt/hostedtoolcache/julia/1.9.3/x64/share/julia/stdlib/v1.9/Test/src/Test.jl:1498 [inlined]
   [11] top-level scope
      @ ~/work/Manifolds.jl/Manifolds.jl/test/manifolds/embedded_torus.jl:11

See here: https://github.com/JuliaManifolds/Manifolds.jl/pull/657 . The v4 version works fine.

ErikQQY commented 1 year ago

The interpolations for MIRK methods are now generated by their specific interpolants instead of SciMLBase.jl ones, but this seems like a bug, I will work on it.

See https://github.com/SciML/BoundaryValueDiffEq.jl/pull/112

avik-pal commented 1 year ago

The multiple shooting PR fixes this. but it might take time to land. @ErikQQY if you want you can pull out the changes there for this into a fresh PR

ErikQQY commented 1 year ago

@avik-pal Sure, I will draft a PR for that