Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Added reduction monte carlo #5

Merged
merged 3 commits into from
Feb 8, 2017
Merged

Added reduction monte carlo #5

merged 3 commits into from
Feb 8, 2017

Conversation

elevien
Copy link
Contributor

@elevien elevien commented Feb 7, 2017

Playing around with ReductionMonteCarloSimulation Type and dispatch for monte_carlo_simulation. prob_func is now a keyword argument.

@ChrisRackauckas
Copy link
Member

Does it need to be a separate function? I think the original MonteCarloSimulation could be modified to have the fieldname changed to solution_data, and have an argument for output_func(sol). I don't think there's a need to make it only work on last(sol), since the user can turn off all intermediate saving by passing save_timeseries=false (since the splatted kwargs are sent to the solver). That would get better performance than creating a full solver and then discarding all but the last point anyways, and would make the output_func more general.

As for making prob_func a kwarg: did you check the @code_warntype to see if there's any type-stability issues? You always have to be careful about changing the type of the argument in a keyword argument because kwargs don't dispatch (at least until JuliaLang/julia#16580 gets merged). I think it may be okay here, but I think it should be checked for stability and whether there's a performance hit first.

If prob_func is fine as a keyword argument performance-wise, then it all could collapse back into one function with output_func=identity as a kwarg.

@ChrisRackauckas
Copy link
Member

Also, you need to make sure you update the tests when you make changes. The CI tests are failing because it's still using prob_func as an optional arg in the 4th test.

@elevien
Copy link
Contributor Author

elevien commented Feb 7, 2017

Ok thanks. Do you mean get rid of ReductionMonteCarloSimulation, or just the dispatch of monte_carlo_simulation? When running monte_carlo_simulation with output_func, it would be nice if one could run simulations until the sample standard deviation is below some desired tolerance. Does it makes sense to implement this within pmap?

@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage remained the same at 55.172% when pulling 72ceb0a on elevien:levien-reduction into e6e553a on JuliaDiffEq:master.

@codecov-io
Copy link

codecov-io commented Feb 8, 2017

Codecov Report

Merging #5 into master will not impact coverage.

@@           Coverage Diff           @@
##           master       #5   +/-   ##
=======================================
  Coverage   55.17%   55.17%           
=======================================
  Files           1        1           
  Lines          29       29           
=======================================
  Hits           16       16           
  Misses         13       13
Impacted Files Coverage Δ
src/DiffEqMonteCarlo.jl 55.17% <81.25%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6e553a...72ceb0a. Read the comment docs.

@ChrisRackauckas ChrisRackauckas merged commit 9376aab into SciML:master Feb 8, 2017
@ChrisRackauckas
Copy link
Member

Thanks!

@elevien elevien deleted the levien-reduction branch February 9, 2017 19:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants