SciML / SciMLExpectations.jl

Fast uncertainty quantification for scientific machine learning (SciML) and differential equations
https://docs.sciml.ai/SciMLExpectations/stable/
Other
65 stars 20 forks source link

Integrals 4 upgrade #148

Closed ArnoStrouwen closed 6 months ago

ArnoStrouwen commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

But the RAT stuff should get resolved.

lxvm commented 7 months ago

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 commented 7 months ago

The adjoint fix is in https://github.com/SciML/RecursiveArrayTools.jl/pull/347

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

lxvm commented 7 months ago

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 https://github.com/SciML/SciMLBase.jl/pull/609 to fix it

ArnoStrouwen commented 7 months ago

@AayushSabharwal Running this test file to the first testset will produce the error locally on linux for me, but not windows: https://github.com/SciML/SciMLExpectations.jl/blob/196532358a90e9fd61c3a811b0b19ea0f7d57b28/test/solve.jl#L1-L50

AayushSabharwal commented 7 months ago

@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 s≈analytical[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 commented 6 months ago

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

ArnoStrouwen commented 6 months ago

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

lxvm commented 6 months ago

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.

ArnoStrouwen commented 6 months ago

@lxvm I think there is still something missing. There are CI differences between linux and windows, why would this nout =1 thing not show up on windows?

I also don't understand what C libraries have to do with there no longer being a "first discretize then optimize" version of the sensitivities. Would the current approach still not just call HCubature.jl, with an extended state?

lxvm commented 6 months ago

I haven't thought about why the nout=1 issue isn't a problem on Windows, but at least it seems like the same version of Integrals is installed. Another observation is that the error is happening in the Cuba.jl extension. I don't have a Windows machine to test with.

The reason I brought up C libraries is because currently Integrals is using the same forward pass for all algorithms (see here), and that code is more restrictive about types than what was previously allowed with direct AD on HCubature.jl. Since SciMLExpectations.jl defines integrands that are not compatible with integration by C libraries, we are currently seeing an error that would be a separate issue were it not for the restrictions on the forward-mode AD. I am working on an Integrals pr to restore direct AD on HCubatureJL pending some fixes for inplace integrands.

ArnoStrouwen commented 6 months ago

Ideally, both sensitivity methods should be available, similar to how ForwardSensitivity and ForwardDiffSensitivity work for differential equation sensitivity: https://docs.sciml.ai/SciMLSensitivity/stable/manual/differential_equation_sensitivities/#Sensitivity-Algorithms. The Integrals AD code should likely also be upstreamed there.

lxvm commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

The new Integrals sadly did not fix the issues.

lxvm commented 6 months ago

@ArnoStrouwen I see two issues, one which gets fixed by https://github.com/SciML/RecursiveArrayTools.jl/pull/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): https://github.com/ArnoStrouwen/SciMLExpectations.jl/pull/9

ArnoStrouwen commented 6 months ago

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

lxvm commented 6 months ago

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. https://github.com/ArnoStrouwen/SciMLExpectations.jl/pull/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 commented 6 months ago

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.