Relax type of alg in ensemble solve for optimization #534
Conversation
Codecov Report
@@ Coverage Diff @@
## master #534 +/- ##
==========================================
+ Coverage 41.07% 42.28% +1.20%
==========================================
Files 53 53
Lines 4051 4063 +12
==========================================
+ Hits 1664 1718 +54
+ Misses 2387 2345 -42
... and 4 files with indirect coverage changes 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
| ensemblealg::BasicEnsembleAlgorithm; | ||
| trajectories, batch_size = trajectories, | ||
| pmap_batch_size = batch_size ÷ 100 > 0 ? batch_size ÷ 100 : 1, kwargs...) | ||
| pmap_batch_size = batch_size ÷ 100 > 0 ? batch_size ÷ 100 : 1, kwargs...) where {A} |
There was a problem hiding this comment.
| pmap_batch_size = batch_size ÷ 100 > 0 ? batch_size ÷ 100 : 1, kwargs...) where {A} | |
| pmap_batch_size = batch_size ÷ 100 > 0 ? batch_size ÷ 100 : 1, kwargs...) where {A <: SciMLAlgorithm} |
I think for this to work it needs a dispatch that's not EnsembleAlgorithm?
There was a problem hiding this comment.
Because of the automatic alg selection in DifferentialEquations.jl?
There was a problem hiding this comment.
I think I could work around this by making a stuct subtype of SciMLAlgorithm and wrap the optimizer's alg in that and handle it in __solve so we might not need to do this then
There was a problem hiding this comment.
Can you double check some cases with this and defaults? Tests seem to pass
src/ensemble/ensemble_problems.jl
Outdated
| EnsembleProblem(prob; prob_func, output_func, reduction, u_init, safetycopy) | ||
| end | ||
|
|
||
| function EnsembleProblem(; prob, |
There was a problem hiding this comment.
I'll use this dispatch for this constructor https://github.com/SciML/Optimization.jl/pull/620/files#diff-6eea19b562ee1839438c6fa37d876324d501594af403caf5c836620ad7e3b793R6
Vaibhavdixit02
force-pushed
the
optensemble
branch
from
b16e2e1 to
23754a8
Compare
src/ensemble/ensemble_problems.jl
Outdated
|
|
||
| function EnsembleProblem(; prob, | ||
| trajectories::Int, | ||
| prob_func, |
There was a problem hiding this comment.
this doesn't define a prob_func?
There was a problem hiding this comment.
Yes, because it'll need to access generated u0s, like https://github.com/SciML/Optimization.jl/pull/620/files#diff-6eea19b562ee1839438c6fa37d876324d501594af403caf5c836620ad7e3b793R6 or as a global variable so
test/runtests.jl
Outdated
| # @time @safetestset "Ensemble Optimization and Nonlinear problems" begin | ||
| # include("downstream/ensemble_nondes.jl") | ||
| # end |
There was a problem hiding this comment.
Yeah it was to show that other tests all pass, to get this one working it needs the dispatches we have been discussing
There was a problem hiding this comment.
I'm lost. What's the version that works with all tests?
There was a problem hiding this comment.
The one without type assertion in the __solve method
There was a problem hiding this comment.
Can you put it this in the form you believe works for everything and we can check the downstream tests and these tests?
Vaibhavdixit02
force-pushed
the
optensemble
branch
from
6575373 to
dea66d1
Compare
This will allow using the existing code for Optimization.jl ensembles