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

Integrals 4 upgrade #148

Merged
merged 23 commits into from
Feb 23, 2024
Merged

Integrals 4 upgrade #148

merged 23 commits into from
Feb 23, 2024

Conversation

ArnoStrouwen
Copy link
Member

@ArnoStrouwen ArnoStrouwen commented Feb 1, 2024

This package is no longer functional because it is not compatible with Integrals version 4.
This makes it unable to use the latest versions of SciMLBase.
However, NonlinearSolve seems to implicitly requite a quite new version of SciMLBase, leading this package to error.

@lxvm did I handle the change to IntegralProblem correctly? Expclidly spiciying iip like IntegralProblem{batch > 0} together with a regular function (not IntegralsFunction) seems to no longer be supported.

@ArnoStrouwen
Copy link
Member Author

There are two regressions:
First, this error in the AD, where a RAT method seems to be missing:
https://github.com/SciML/SciMLExpectations.jl/actions/runs/7742545973/job/21111921185?pr=148#step:6:714
Second, another failure involving RAT failure only on mac and linux, but curiously not on windows:
https://github.com/SciML/SciMLExpectations.jl/actions/runs/7742545973/job/21111920570?pr=148#step:6:669
@AayushSabharwal do you have any idea how to resolve these?

Then there are also tons of invalidations:
https://github.com/SciML/SciMLExpectations.jl/actions/runs/7742545997/job/21111920908?pr=148#step:8:124
@ChrisRackauckas do you have any idea why these show up?

@ChrisRackauckas
Copy link
Member

Static.jl's invalidations are pretty essential. I wouldn't worry about those.

But the RAT stuff should get resolved.

src/expectation.jl Outdated Show resolved Hide resolved
src/expectation.jl Outdated Show resolved Hide resolved
@lxvm
Copy link
Contributor

lxvm commented Feb 2, 2024

This package is no longer functional because it is not compatible with Integrals version 4.

I see a CI error in src/expectations.jl:66, which is in a routine for a batched integrand and about trying to reshape a float that points to here, which means the error is happening when indexing the EnsembleSolution. Why is a batched integrand evaluating a scalar EnsembleSolution?

It's worth mentioning that Integrals v4 is different in that Cuba returns a scalar only when the integrand is scalar here. Also, another rule of thumb is that batched integrands will loose their outermost axis, so a batched integrand that returns vectors will integrate to a scalar and if it returns matrices will integrate to a vector. I hope this helps

I would also encourage using an inplace (Batch)IntegralFunction since that lets you control the shape of the integrand output array, instead of nout which only allows vectors.

@AayushSabharwal
Copy link
Member

The adjoint fix is in SciML/RecursiveArrayTools.jl#347

I can't replicate the second error. Do you have an MWE?

@lxvm
Copy link
Contributor

lxvm commented Feb 3, 2024

Expclidly spiciying iip like IntegralProblem{batch > 0} together with a regular function (not IntegralsFunction) seems to no longer be supported.

@ArnoStrouwen this seems like a regression, so I opened SciML/SciMLBase.jl#609 to fix it

@ArnoStrouwen ArnoStrouwen reopened this Feb 5, 2024
@ArnoStrouwen
Copy link
Member Author

@AayushSabharwal
Running this test file to the first testset will produce the error locally on linux for me, but not windows:

using Test, TestExtras,
SciMLExpectations, OrdinaryDiffEq, Distributions,
Integrals, IntegralsCubature, IntegralsCuba,
StaticArrays, ComponentArrays, Random
quadalgs = [
HCubatureJL(),
CubatureJLh(),
CubatureJLp(),
CubaSUAVE(),
CubaDivonne(),
CubaCuhre(),
]
quadalgs_batch = [CubatureJLh(), CubatureJLp(), CubaSUAVE(), CubaDivonne(), CubaCuhre()]
# batchmode = [EnsembleSerial(), EnsembleThreads()]#, EnsembleGPUArray()]
@testset "DiffEq Expectation Solve" begin
function eom!(du, u, p, t, A)
@inbounds begin du .= A * u end
nothing
end
u0 = [1.0, 1.0]
tspan = (0.0, 3.0)
p = [1.0; 2.0]
A = [0.0 1.0; -p[1] -p[2]]
prob = ODEProblem((du, u, p, t) -> eom!(du, u, p, t, A), u0, tspan, p)
u0s_dist = (Uniform(1, 10), truncated(Normal(3.0, 1), 0.0, 6.0))
gd = GenericDistribution(u0s_dist...)
cov(x, u, p) = x, p
sm = SystemMap(prob, Tsit5(), save_everystep = false)
analytical = (exp(A * tspan[end]) * [mean(d) for d in u0s_dist])
@testset "Scalar Observable (nout = 1)" begin
g(sol, p) = sol[1, end]
exprob = ExpectationProblem(sm, g, cov, gd)
for alg in quadalgs
@test solve(exprob, Koopman(); quadalg = alg, ireltol = 1e-3, iabstol = 1e-3).u[1]analytical[1] rtol=1e-2
# @constinferred solve(exprob, Koopman(); quadalg = alg)[1] # Commented b/c no "broken" inferred macros and is not stable due to Quadrature.jl
if alg quadalgs_batch
s = solve(exprob, Koopman(); quadalg = alg, ireltol = 1e-3, iabstol = 1e-3,
batch = 20).u[1]
@test sanalytical[1] rtol=1e-2
# @constinferred solve(exprob, Koopman(); quadalg = alg, batch = 5)[1] # Commented b/c no "broken" inferred macros and is not stable due to Quadrature.jl
end
end
@test solve(exprob, MonteCarlo(10000)).u[1]analytical[1] rtol=1e-2
@constinferred solve(exprob, MonteCarlo(10000))
end

@AayushSabharwal
Copy link
Member

AayushSabharwal commented Feb 14, 2024

@ArnoStrouwen
Running the test file produces an error outside of RAT for me:

julia> for alg in quadalgs
                    @test solve(exprob, Koopman(); quadalg = alg, ireltol = 1e-3, iabstol = 1e-3).u[1]analytical[1] rtol=1e-2
                    # @constinferred solve(exprob, Koopman(); quadalg = alg)[1]  # Commented b/c no "broken" inferred macros and is not stable due to Quadrature.jl
                    if alg  quadalgs_batch
                        s = solve(exprob, Koopman(); quadalg = alg, ireltol = 1e-3, iabstol = 1e-3,
                        batch = 20).u[1]
                        @test sanalytical[1] rtol=1e-2
                        # @constinferred solve(exprob, Koopman(); quadalg = alg, batch = 5)[1]  # Commented b/c no "broken" inferred macros and is not stable due to Quadrature.jl
                    end
                end
┌ Warning: `nout` and `batch` keywords are deprecated in favor of inplace `IntegralFunction`s or `BatchIntegralFunction`s. See the updated Integrals.jl documentation for details.
└ @ SciMLBase ~/.julia/packages/SciMLBase/QSc1r/src/problems/basic_problems.jl:476
Error During Test at REPL[31]:2
  Test threw exception
  Expression: ((solve(exprob, Koopman(); quadalg = alg, ireltol = 0.001, iabstol = 0.001)).u[1], analytical[1], rtol = 0.01)
  MethodError: objects of type SystemMap{ODEProblem{Vector{Float64}, Tuple{Float64, Float64}, true, Vector{Float64}, ODEFunction{true, SciMLBase.AutoSpecialize, var"#1#2", LinearAlgebra.UniformScaling{Bool}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, typeof(SciMLBase.DEFAULT_OBSERVED), Nothing, Nothing}, Base.Pairs{Symbol, Union{}, Tuple{}, @NamedTuple{}}, SciMLBase.StandardODEProblem}, Tsit5{typeof(OrdinaryDiffEq.trivial_limiter!), typeof(OrdinaryDiffEq.trivial_limiter!), Static.False}, EnsembleThreads, Base.Pairs{Symbol, Bool, Tuple{Symbol}, @NamedTuple{save_everystep::Bool}}} are not callable
  Stacktrace:
    [1] (::SciMLExpectations.var"#23#24"{GenericDistribution{SciMLExpectations.var"#pdf_func#10"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SciMLExpectations.var"#rand_func#12"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SVector{2, Float64}, SVector{2, Float64}}, typeof(cov), typeof(g), SystemMap{ODEProblem{Vector{Float64}, Tuple{Float64, Float64}, true, Vector{Float64}, ODEFunction{true, SciMLBase.AutoSpecialize, var"#1#2", LinearAlgebra.UniformScaling{Bool}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, typeof(SciMLBase.DEFAULT_OBSERVED), Nothing, Nothing}, @Kwargs{}, SciMLBase.StandardODEProblem}, Tsit5{typeof(OrdinaryDiffEq.trivial_limiter!), typeof(OrdinaryDiffEq.trivial_limiter!), Static.False}, EnsembleThreads, @Kwargs{save_everystep::Bool}}})(x::SVector{2, Float64}, p::RecursiveArrayTools.ArrayPartition{Float64, Tuple{Vector{Float64}, Vector{Float64}}})
      @ SciMLExpectations ~/.julia/packages/SciMLExpectations/gB7GE/src/expectation.jl:48
    [2] (::IntegralFunction{false, SciMLBase.FullSpecialize, SciMLExpectations.var"#23#24"{GenericDistribution{SciMLExpectations.var"#pdf_func#10"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SciMLExpectations.var"#rand_func#12"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SVector{2, Float64}, SVector{2, Float64}}, typeof(cov), typeof(g), SystemMap{ODEProblem{Vector{Float64}, Tuple{Float64, Float64}, true, Vector{Float64}, ODEFunction{true, SciMLBase.AutoSpecialize, var"#1#2", LinearAlgebra.UniformScaling{Bool}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, typeof(SciMLBase.DEFAULT_OBSERVED), Nothing, Nothing}, @Kwargs{}, SciMLBase.StandardODEProblem}, Tsit5{typeof(OrdinaryDiffEq.trivial_limiter!), typeof(OrdinaryDiffEq.trivial_limiter!), Static.False}, EnsembleThreads, @Kwargs{save_everystep::Bool}}}, Nothing})(::SVector{2, Float64}, ::Vararg{Any})
      @ SciMLBase ~/.julia/packages/SciMLBase/QSc1r/src/scimlfunctions.jl:2199

ChrisRackauckas and others added 2 commits February 18, 2024 08:58
Co-authored-by: Lorenzo Van Munoz <[email protected]>
Co-authored-by: Lorenzo Van Munoz <[email protected]>
Comment on lines 29 to 34
function (sm::SystemMap{DT})(u0, p) where {DT}
prob::DT = remake(sm.prob,
u0 = convert(typeof(sm.prob.u0), u0),
p = convert(typeof(sm.prob.p), p))
solve(prob, sm.alg; sm.kwargs...)
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this call removed? Isn't this what's causing the failure?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might have deleted the wrong one:
https://github.com/SciML/SciMLExpectations.jl/blob/master/test/interface.jl#L8-L13
But it is strange I and CI don't get errors about this piece missing, while @AayushSabharwal does. I guess safetestsets are not completely separated?
I'll switch it around and see what it gives.

@ChrisRackauckas
Copy link
Member

The bump of the high level docs SciML/SciMLDocs#217 is a bit blocked on this so we should figure out the end solution here.

@ArnoStrouwen
Copy link
Member Author

@AayushSabharwal Could you have another look if you still have the same issue?

@lxvm
Copy link
Contributor

lxvm commented Feb 18, 2024

As I see it the two remaining issues in the CI are:

  1. A problem in the nout=1 case, which I could try to fix
  2. A ForwardDiff.jl error when calling HCubatureJL since I no longer do direct forward-mode AD on those solvers and instead redirect every solver through the same forward pass, which is causing an error since the integrand is not compatible with C libraries (it returns an array of arrays instead of an array of numbers). I see two ways around this:
  • flattening the integrand so that instead of returning arrays of arrays it returns a flat array with the same data.
  • Changing Integrals.jl so that the forward pass for C libraries is restricted to C libraries, while also correctly handling some type changes for in-place integral functions that will be used with Julia libraries. I think this should be done anyways.

Any thoughts? Making the integrand compatible with C libraries would also be good to have if the user is allowed to pick the algorithm.

@lxvm
Copy link
Contributor

lxvm commented Feb 18, 2024

I'm hoping that a patch will be enough to get the tests passing again.

Ultimately, it would be great to unify the sensitivity algorithms upstream, but it is a substantial project that should probably be tackled at the same time as adding differentiation w.r.t. limits of integration because of any complications which may arise. I'm in over my head this semester but I might have time over the summer.

@lxvm
Copy link
Contributor

lxvm commented Feb 18, 2024

I've made relevant prs that should get these tests to pass. It would be worth considering deprecating nout and batch in another pr, as it could require breaking changes

@ArnoStrouwen
Copy link
Member Author

I think this PR is already breaking since it removes the IntegralsCubature packages and replaces them with Cubature? Might as well get all breaking stuff out of the way at once.

@AayushSabharwal
Copy link
Member

AayushSabharwal commented Feb 19, 2024

It's not the same issue, but it still doesn't error in RAT. The line that errors is:

@test solve(exprob, Koopman(); quadalg = alg, ireltol = 1e-3, iabstol = 1e-3).u[1]analytical[1] rtol=1e-2

With error:

┌ Warning: `nout` and `batch` keywords are deprecated in favor of inplace `IntegralFunction`s or `BatchIntegralFunction`s. See the updated Integrals.jl documentation for details.
└ @ SciMLBase ~/.julia/packages/SciMLBase/wVDwN/src/problems/basic_problems.jl:476
Error During Test at REPL[18]:2
  Test threw exception
  Expression: ((solve(exprob, Koopman(); quadalg = alg, ireltol = 0.001, iabstol = 0.001)).u[1], analytical[1], rtol = 0.01)
  AssertionError: prob.f isa IntegralFunction
  Stacktrace:
    [1] __solvebp_call(prob::IntegralProblem{false, RecursiveArrayTools.ArrayPartition{Float64, Tuple{Vector{Float64}, Vector{Float64}}}, BatchIntegralFunction{false, SciMLBase.FullSpecialize, SciMLExpectations.var"#23#24"{GenericDistribution{SciMLExpectations.var"#pdf_func#10"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SciMLExpectations.var"#rand_func#12"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SVector{2, Float64}, SVector{2, Float64}}, typeof(cov), typeof(g), SystemMap{ODEProblem{Vector{Float64}, Tuple{Float64, Float64}, true, Vector{Float64}, ODEFunction{true, SciMLBase.AutoSpecialize, var"#1#2", LinearAlgebra.UniformScaling{Bool}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, typeof(SciMLBase.DEFAULT_OBSERVED), Nothing, Nothing}, @Kwargs{}, SciMLBase.StandardODEProblem}, Tsit5{typeof(OrdinaryDiffEq.trivial_limiter!), typeof(OrdinaryDiffEq.trivial_limiter!), Static.False}, EnsembleThreads, @Kwargs{save_everystep::Bool}}}, Nothing}, Tuple{SVector{2, Float64}, SVector{2, Float64}}, @Kwargs{}}, alg::HCubatureJL{typeof(LinearAlgebra.norm)}, sensealg::Integrals.ReCallVJP{Integrals.ZygoteVJP}, domain::Tuple{SVector{2, Float64}, SVector{2, Float64}}, p::RecursiveArrayTools.ArrayPartition{Float64, Tuple{Vector{Float64}, Vector{Float64}}}; reltol::Float64, abstol::Float64, maxiters::Int64)
      @ Integrals ~/.julia/packages/Integrals/CazE7/src/Integrals.jl:151
    [2] __solvebp_call(::Integrals.IntegralCache{false, BatchIntegralFunction{false, SciMLBase.FullSpecialize, SciMLExpectations.var"#23#24"{GenericDistribution{SciMLExpectations.var"#pdf_func#10"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SciMLExpectations.var"#rand_func#12"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SVector{2, Float64}, SVector{2, Float64}}, typeof(cov), typeof(g), SystemMap{ODEProblem{Vector{Float64}, Tuple{Float64, Float64}, true, Vector{Float64}, ODEFunction{true, SciMLBase.AutoSpecialize, var"#1#2", LinearAlgebra.UniformScaling{Bool}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, typeof(SciMLBase.DEFAULT_OBSERVED), Nothing, Nothing}, @Kwargs{}, SciMLBase.StandardODEProblem}, Tsit5{typeof(OrdinaryDiffEq.trivial_limiter!), typeof(OrdinaryDiffEq.trivial_limiter!), Static.False}, EnsembleThreads, @Kwargs{save_everystep::Bool}}}, Nothing}, Tuple{SVector{2, Float64}, SVector{2, Float64}}, RecursiveArrayTools.ArrayPartition{Float64, Tuple{Vector{Float64}, Vector{Float64}}}, @Kwargs{}, HCubatureJL{typeof(LinearAlgebra.norm)}, Integrals.ReCallVJP{Integrals.ZygoteVJP}, @Kwargs{reltol::Float64, abstol::Float64, maxiters::Int64}, Nothing}, ::HCubatureJL{typeof(LinearAlgebra.norm)}, ::Vararg{Any}; kwargs::@Kwargs{reltol::Float64, abstol::Float64, maxiters::Int64})
      @ Integrals ~/.julia/packages/Integrals/CazE7/src/common.jl:115
    [3] __solvebp(::Integrals.IntegralCache{false, BatchIntegralFunction{false, SciMLBase.FullSpecialize, SciMLExpectations.var"#23#24"{GenericDistribution{SciMLExpectations.var"#pdf_func#10"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SciMLExpectations.var"#rand_func#12"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SVector{2, Float64}, SVector{2, Float64}}, typeof(cov), typeof(g), SystemMap{ODEProblem{Vector{Float64}, Tuple{Float64, Float64}, true, Vector{Float64}, ODEFunction{true, SciMLBase.AutoSpecialize, var"#1#2", LinearAlgebra.UniformScaling{Bool}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, typeof(SciMLBase.DEFAULT_OBSERVED), Nothing, Nothing}, @Kwargs{}, SciMLBase.StandardODEProblem}, Tsit5{typeof(OrdinaryDiffEq.trivial_limiter!), typeof(OrdinaryDiffEq.trivial_limiter!), Static.False}, EnsembleThreads, @Kwargs{save_everystep::Bool}}}, Nothing}, Tuple{SVector{2, Float64}, SVector{2, Float64}}, RecursiveArrayTools.ArrayPartition{Float64, Tuple{Vector{Float64}, Vector{Float64}}}, @Kwargs{}, HCubatureJL{typeof(LinearAlgebra.norm)}, Integrals.ReCallVJP{Integrals.ZygoteVJP}, @Kwargs{reltol::Float64, abstol::Float64, maxiters::Int64}, Nothing}, ::Vararg{Any}; kwargs::@Kwargs{reltol::Float64, abstol::Float64, maxiters::Int64})
      @ Integrals ~/.julia/packages/Integrals/CazE7/src/Integrals.jl:65
    [4] solve!(cache::Integrals.IntegralCache{false, BatchIntegralFunction{false, SciMLBase.FullSpecialize, SciMLExpectations.var"#23#24"{GenericDistribution{SciMLExpectations.var"#pdf_func#10"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SciMLExpectations.var"#rand_func#12"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SVector{2, Float64}, SVector{2, Float64}}, typeof(cov), typeof(g), SystemMap{ODEProblem{Vector{Float64}, Tuple{Float64, Float64}, true, Vector{Float64}, ODEFunction{true, SciMLBase.AutoSpecialize, var"#1#2", LinearAlgebra.UniformScaling{Bool}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, typeof(SciMLBase.DEFAULT_OBSERVED), Nothing, Nothing}, @Kwargs{}, SciMLBase.StandardODEProblem}, Tsit5{typeof(OrdinaryDiffEq.trivial_limiter!), typeof(OrdinaryDiffEq.trivial_limiter!), Static.False}, EnsembleThreads, @Kwargs{save_everystep::Bool}}}, Nothing}, Tuple{SVector{2, Float64}, SVector{2, Float64}}, RecursiveArrayTools.ArrayPartition{Float64, Tuple{Vector{Float64}, Vector{Float64}}}, @Kwargs{}, HCubatureJL{typeof(LinearAlgebra.norm)}, Integrals.ReCallVJP{Integrals.ZygoteVJP}, @Kwargs{reltol::Float64, abstol::Float64, maxiters::Int64}, Nothing})
      @ Integrals ~/.julia/packages/Integrals/CazE7/src/common.jl:105
    [5] solve(prob::IntegralProblem{false, RecursiveArrayTools.ArrayPartition{Float64, Tuple{Vector{Float64}, Vector{Float64}}}, BatchIntegralFunction{false, SciMLBase.FullSpecialize, SciMLExpectations.var"#23#24"{GenericDistribution{SciMLExpectations.var"#pdf_func#10"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SciMLExpectations.var"#rand_func#12"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SVector{2, Float64}, SVector{2, Float64}}, typeof(cov), typeof(g), SystemMap{ODEProblem{Vector{Float64}, Tuple{Float64, Float64}, true, Vector{Float64}, ODEFunction{true, SciMLBase.AutoSpecialize, var"#1#2", LinearAlgebra.UniformScaling{Bool}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, typeof(SciMLBase.DEFAULT_OBSERVED), Nothing, Nothing}, @Kwargs{}, SciMLBase.StandardODEProblem}, Tsit5{typeof(OrdinaryDiffEq.trivial_limiter!), typeof(OrdinaryDiffEq.trivial_limiter!), Static.False}, EnsembleThreads, @Kwargs{save_everystep::Bool}}}, Nothing}, Tuple{SVector{2, Float64}, SVector{2, Float64}}, @Kwargs{}}, alg::HCubatureJL{typeof(LinearAlgebra.norm)}; kwargs::@Kwargs{reltol::Float64, abstol::Float64, maxiters::Int64})
      @ Integrals ~/.julia/packages/Integrals/CazE7/src/common.jl:101
    [6] integrate(quadalg::HCubatureJL{typeof(LinearAlgebra.norm)}, adalg::NonfusedAD, f::Function, lb::SVector{2, Float64}, ub::SVector{2, Float64}, p::RecursiveArrayTools.ArrayPartition{Float64, Tuple{Vector{Float64}, Vector{Float64}}}; nout::Int64, batch::Int64, kwargs::@Kwargs{reltol::Float64, abstol::Float64, maxiters::Int64})
      @ SciMLExpectations ~/Julia/SciML/ArnoSciMLExp/src/expectation.jl:230
    [7] integrate
      @ ~/Julia/SciML/ArnoSciMLExp/src/expectation.jl:225 [inlined]
    [8] solve(::ExpectationProblem{SystemMap{ODEProblem{Vector{Float64}, Tuple{Float64, Float64}, true, Vector{Float64}, ODEFunction{true, SciMLBase.AutoSpecialize, var"#1#2", LinearAlgebra.UniformScaling{Bool}, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, typeof(SciMLBase.DEFAULT_OBSERVED), Nothing, Nothing}, @Kwargs{}, SciMLBase.StandardODEProblem}, Tsit5{typeof(OrdinaryDiffEq.trivial_limiter!), typeof(OrdinaryDiffEq.trivial_limiter!), Static.False}, EnsembleThreads, @Kwargs{save_everystep::Bool}}, typeof(g), typeof(cov), GenericDistribution{SciMLExpectations.var"#pdf_func#10"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SciMLExpectations.var"#rand_func#12"{Tuple{Uniform{Float64}, Truncated{Normal{Float64}, Continuous, Float64, Float64, Float64}}}, SVector{2, Float64}, SVector{2, Float64}}, RecursiveArrayTools.ArrayPartition{Float64, Tuple{Vector{Float64}, Vector{Float64}}}}, ::Koopman{NonfusedAD}; maxiters::Int64, batch::Int64, quadalg::HCubatureJL{typeof(LinearAlgebra.norm)}, ireltol::Float64, iabstol::Float64, kwargs::@Kwargs{})
      @ SciMLExpectations ~/Julia/SciML/ArnoSciMLExp/src/expectation.jl:216

@ArnoStrouwen
Copy link
Member Author

The new Integrals sadly did not fix the issues.

@lxvm
Copy link
Contributor

lxvm commented Feb 22, 2024

@ArnoStrouwen I see two issues, one which gets fixed by SciML/RecursiveArrayTools.jl#355. I also made a pr to this pr that you can merge that should fix all the issues and is a full upgrade to the new interface (it has a workaround for the first): ArnoStrouwen#9

@ArnoStrouwen
Copy link
Member Author

The differentiation error was fixed now, but there are remaining errors. Docs are OK now too.

@lxvm
Copy link
Contributor

lxvm commented Feb 23, 2024

I took a look at updating the broken interface tests since I hadn't addressed them before and made a new pr to this pr that passes the CI. ArnoStrouwen#12

One change is that I removed a type stability test on the constructor of the batch integrand, which does an ODE solve to get the type of the output that seems to be type-unstable. This pattern of tests for type stability of internal interfaces without testing the type stability of functions users will call seems like a code smell to me. As an example of a type instability that would be missed without top-level tests, see: https://github.com/SciML/DiffEqBase.jl/blob/6283b76a6ff95f5ca9270ebec3cbe72aa86ea157/src/solve.jl#L975. In that function, union splitting gives up because two variables u0, p are reassigned and may change types. As a result, the inferred result is Any, which propagates to the tests for this package when a simple test upstream could have caught the issue and been used to rename the variables to fix the instability.

@ChrisRackauckas
Copy link
Member

https://github.com/SciML/DiffEqBase.jl/blob/6283b76a6ff95f5ca9270ebec3cbe72aa86ea157/src/solve.jl#L975. In that function, union splitting gives up because two variables u0, p are reassigned and may change types. As a result, the inferred result is Any, which propagates to the tests for this package when a simple test upstream could have caught the issue and been used to rename the variables to fix the instability.

That shouldn't do anything after optimizations since the SSA IR will lower those to different variables.

@ChrisRackauckas ChrisRackauckas merged commit eec17b6 into SciML:master Feb 23, 2024
7 of 8 checks passed
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.

4 participants