-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Integrals 4 upgrade #148
Conversation
d144a9b
to
02c1f9a
Compare
There are two regressions: Then there are also tons of invalidations: |
Static.jl's invalidations are pretty essential. I wouldn't worry about those. But the RAT stuff should get resolved. |
I see a CI error in 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 |
The adjoint fix is in SciML/RecursiveArrayTools.jl#347 I can't replicate the second error. Do you have an MWE? |
@ArnoStrouwen this seems like a regression, so I opened SciML/SciMLBase.jl#609 to fix it |
@AayushSabharwal SciMLExpectations.jl/test/solve.jl Lines 1 to 50 in 1965323
|
@ArnoStrouwen 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 |
Co-authored-by: Lorenzo Van Munoz <[email protected]>
Co-authored-by: Lorenzo Van Munoz <[email protected]>
src/system_utils.jl
Outdated
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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. |
@AayushSabharwal Could you have another look if you still have the same issue? |
As I see it the two remaining issues in the CI are:
Any thoughts? Making the integrand compatible with C libraries would also be good to have if the user is allowed to pick the algorithm. |
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. |
I've made relevant prs that should get these tests to pass. It would be worth considering deprecating |
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. |
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 |
The new Integrals sadly did not fix the issues. |
@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 |
no trivial batching
The differentiation error was fixed now, but there are remaining errors. Docs are OK now too. |
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 |
That shouldn't do anything after optimizations since the SSA IR will lower those to different variables. |
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 likeIntegralProblem{batch > 0}
together with a regular function (notIntegralsFunction
) seems to no longer be supported.