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

Problem Interface #35

Closed
wants to merge 7 commits into from
Closed

Problem Interface #35

wants to merge 7 commits into from

Conversation

aml5600
Copy link
Contributor

@aml5600 aml5600 commented Oct 16, 2020

  • creates an AbstractUncertaintyProblem abstract type and ExpectationProblem struct
  • adds solve methods for ExpectationProblem using Koopman and MonteCarlo
  • moves organizational logic out of solve and into problem construction
  • makes organizational logic more general...should be able to mix multivariate, univariate, and scalar distributions/values within the state and parameter arrays
  • added uncertainty_utils.jl for helper functions

Relevant tests pass...The quadrature methods do not seem to work with ForwardDiff as the dual variables "enter" into the problem's lower/upper bounds. The construction of the problem should make it easy to setup for various problem types, this one should handle what I would like it to do at the moment!

src/koopman.jl Outdated
- quadalg: quadrature algorithm to use (default HCubatureJL())
- ireltol, iabstol: integration relative and absolute tolerances (default 1e-2, 1e-2)
"""
function solve(prob::ExpectationProblem, expalg::Koopman, args...;
Copy link
Member

Choose a reason for hiding this comment

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

DiffEqBase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you elaborate?

Copy link
Member

Choose a reason for hiding this comment

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

DiffEqBase.solve

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gotcha, thanks!

src/koopman.jl Outdated
f0 = map(prob.f0_func, zip(_x,_p))
end

fq .= Ug .* f0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ChrisRackauckas this causes HCubatureJL to fail for nout=1...if I switch the integrand to oop, it passes....

I dug into it, and it executes the exact same quadrature evaluation points and returning integrand values for both directly returning a scalar and in-place for a length=1 array. The quadrature alg seems to keep evaluating more points for the latter therefore returning a different value.

Any ideas?
FYI @agerlach

Copy link
Contributor

Choose a reason for hiding this comment

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

HCubature will internally call different algorithms depending on if scalar bounds or array. This may explain it??? See hcubature vs hquadrature https://github.com/JuliaMath/HCubature.jl

@aml5600
Copy link
Contributor Author

aml5600 commented Oct 21, 2020

So I cannot get ForwardDiff to work with anything but HCubatueJL and that is with manually setting the upper and lower bounds. If the dual variable enters in through there then it breaks on HCubtaure.jl line 127, it seems this is a known problem (see here ).

I would imagine that the only reason the original tests pass is because the AD does not enter the bounds via the specific problem we are testing.

On the bright side, Zygote has no problems with the "full AD" problem!

@aml5600 aml5600 changed the title [WIP] Problem Interface Problem Interface Oct 21, 2020
@ChrisRackauckas
Copy link
Member

So I cannot get ForwardDiff to work with anything but HCubatueJL and that is with manually setting the upper and lower bounds. If the dual variable enters in through there then it breaks on HCubtaure.jl line 127, it seems this is a known problem (see here ).

HCubature should get a PR to fix this. I think we can merge without that, but the tests should be set to properly mark which are broken and which are not (and in the current setup that will require some conditionals)

@agerlach
Copy link
Contributor

@aml5600 can you update the readme to reflect the interface change? Also, SciMLTutorials SciML/SciMLTutorials.jl#343

How does or will this interface work w/ convenience methods like centralmoments()

@aml5600 aml5600 closed this May 7, 2021
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.

3 participants