-
-
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
[WIP] ExpectationProblem interface #55
[WIP] ExpectationProblem interface #55
Conversation
…ing and type stable
…batch arguments to zygote adjoints
… store in g instead of S
…lach/DiffEqUncertainty.jl into agerlach/ExpectationProblem
@ChrisRackauckas I would like to get your thoughts on this before pushing forward. |
I think this is good. I don't quite see the need for |
@aml5600 and I debated on whether or not to include a non-DE interface. I wanted to have it for a couple of trivial applications, but I can see not wanting to maintain that. Alternatively, if we only had a DE interface, on could simply get around it by simulating from from t = 0 to 0. RE: |
I think the issue is always that it confuses people when you have a non-limitation which becomes a documentation limitation because of practice.
If it's just functions of |
@ChrisRackauckas I've been discussing with @agerlach about getting involved in the development of this and I wanted to put on your radar that I'm available to help build up tests or address other issues as this pull request and library move forward |
@time @testset "ProbInts tests" begin include("probints.jl") end | ||
@time @testset "Koopman Tests" begin include("koopman.jl") end | ||
#TODO reenable tests | ||
# @time @safetestset "ProbInts tests" begin include("probints.jl") 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.
what happened to the probints one?
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.
These tests took a long time to run locally, so I temp commented them out.
I dug through this, and I think we can just merge it if it gets rebased and we figure out why the probints tests were commented out. Of course more can keep being done, but I think this is a big enough improvement to just go for it. |
This is not a feature parity yet, so I'm hesitant to merge as is. Looking of the todo list, it looks like |
@jmurphy6895 Adding central moment support via a |
@ChrisRackauckas We never settled on the interface wrt |
|
||
# Type Piracy, should upstream | ||
Base.eltype(K::UnivariateKDE) = eltype(K.density) | ||
Base.minimum(K::UnivariateKDE) = minimum(K.x) | ||
Base.maximum(K::UnivariateKDE) = maximum(K.x) | ||
Base.extrema(K::UnivariateKDE) = minimum(K), maximum(K) | ||
|
||
Base.minimum(d::AbstractMvNormal) = fill(-Inf, length(d)) |
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.
These lines are technically type piracy right? Also, they might not be correct for any AbstractNormal
that accepts singular covariance matrices, e.g., Diagonal([0, 1])
does not have infinite support for the first variable.
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.
Yes, the UnivariateKDE
parts should be upstreamed. Base.minimum(d::AbstractMvNormal)
was upstreamed in Distributions in JuliaStats/Distributions.jl#1319, probably need to double check the PR discussion to make sure a bug wasn't introduced.
Base.maximum(d::AbstractMvNormal) = fill(Inf, length(d)) | ||
Base.extrema(d::AbstractMvNormal) = minimum(d), maximum(d) | ||
|
||
Base.minimum(d::Product) = minimum.(d.v) |
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.
These are already defined in Distributions
https://github.com/JuliaStats/Distributions.jl/blob/master/src/multivariate/product.jl#L43
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 had these here as a stop gap until this PR when through JuliaStats/Distributions.jl#1319
|
||
if prob.nout == 1 # TODO fix upstream in quadrature, expected sizes depend on quadrature method is requires different copying based on nout > 1 | ||
set_result! = @inline function(dx, sol) | ||
dx[:] .= sol[:] |
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'm not sure about the semantics of sol[:]
, but it looks like there are unnecessary allocations from the [:]
here.
Since they are marked @inline
, I guess performance matters here?
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.
set_result!
ends up formating the solution of an EnsembleSolve
to the format required by Quadrature.jl. This is getting called once per batch integrand evaluation. I'm not sure how to clean up the allocation.
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.
This also highlights an interface issue with Quadrature (which I am probably guilty of creating), i.e. we shouldn't really need separate set_result!
for scalar and vector valued integrands.
Actions from our call today:
|
Why does
wouldn't it be enough for |
Consider the ballistic projectile example in the OP. Here, the dynamics are best expressed in cartesian coordinates consisting of a down range and vertical position and corresponding velocities. If you have uncertainty in elevation angle you need to do a change of variables into the state space. However, this process does not effect the entire state-space as the initial launch location is not uncertain. So, if you discretize the uncertainty space, you need a way to map that to the subset of the state-space that is effected in addition to the subset of the parameter space if needed. Here, I map uncertain elevation and exit velocity to the velocity states while leaving position unchanged. I'm not sure if you can do this in-place. The # Define Distribution, can now handle any univariate/MV distribution!
# defined as initial velocity, elevation, CdS
dist = Product([Uniform(13.0, 15.0), Uniform(.9*π/4, 1.1*π/4), Truncated(Normal(1.0, .02), .9, 1.1)])
# Define function of how to change state/param given point in uncertainty space
function h(q, u0, p)
# q = [velocity, elevation, CdS]
u0.ẋ = q[1]*cos(q[2])
u0.ż = q[1]*cos(q[2])
p.CdS = q[3]
u0, p
end |
I played around with removing |
This WIP PR is meant to start discussing interface updates and a general restructuring of DEU. NOTE: the interface described below is implemented w/ tests. However, it is not at feature parity. Mainly, I have not defined the integrand yet to support batch evaluation nor performed extensive testing for AD.
Big shout out to @aml5600 for his help.
This addresses, #48, #44, #34,
The Problems
The interface of the current release for
expectation
requires passing two arrays for the initial conditions and parameters that mix number and distribution types, e.g.1st, one defines the system
then defines, a new array of deterministic and uncertain values for the initial conditions and parameters
Problem 1: This is not type stable due to requiring
Array{Any,1}
for the distributions.Problem 2 Internally the
remake
call forces the initial condition and parameters to beArray
. This fails if the ODE is written for something like ComponentArrays. Here, all the information foru0
andps
in theDEproblem
is lost.Problem 3 Internally this assumes all distributions are independent so the joint can be formed as the product of pdfs
Proposed New Interface
We now have problem/solve workflow like the rest of SciML by introducing the
ExpectationProblem
. This essentially stores all the functions and the distribution required to solve problems of the form: ∫ g(S(h(x,u0,p)))*pdf(d,x)dx.Take 𝕏, 𝕌, ℙ, as the uncertainty, state, and parameters spaces, respectively:
pdf
andextrema
To define, S, we introduce
SystemMap
as a callable object that wrapsremake -> solve
, e.g.is equivalent to
Doing this has 3 primary benefits 1) We can now support arbitrary callable objects for the system map and 2) It prevents downstream
args.../kwargs...
collisions, by isolating the DE solve arguments in theSystemMap
3) it ensures proper conversion to the correctu0
,p
types defined by the ODEProblem.Example 1
Example 2
Because the
ExpectationProblem
just operates on callable objects we can also solve for expectation of arbitrary functionsArbitrary Distribution Support
What if a user has some complicated distribution they want to consider that is not supported by Distributions.jl? For that we introduce the
GenericDistribution
that implements the requiredDistributions.jl
interface for usage in DiffEqUncertainty. The user just supplies a function for computing the pdf, arand()
function (if using MonteCarlo), and lower/upper bounds.A trivial example looks like
Non-Quadrature.jl integration
Will return the integrand required to solve the Koopman Expectation problem. The user can then use non-Quadrature.jl integration methods.
Other Changes of Note
Array
. They now will match the types ofprob.u0
andprob.p
. This enables its usage for system written using <:AbstractArraysTo Do
CentralMomentProblem