Closed dreycenfoiles closed 11 months ago
Hey, this is really cool and it's sad to see it just live as a draft. Is there an intention to complete this?
Absolutely! I apologize that I haven't finished this; I honestly completely forgot about it. I'll probably have some time to do some more this weekend.
Sorry for the delay, but I think the PR is finally ready to look over. Please let me know your thoughts if you have the time and I'd be happy to make the necessary changes.
Overall it looks good, but just needs a few bikeshedding tweaks in order to be ready to merge. We need to be careful that things specialize and exporting 2-3 letter words is generally not a good idea for something that's widely used, so we need to just change a few surface level things.
I think I was able to address your concerns. Is there anything else you'd like me to change?
I think this looks great! It may need to wait for https://github.com/SciML/Surrogates.jl/pull/441 to fix tests on master and rebase on top of that, but I think this is looking ready to merge if tests pass.
Merging #435 (ea4eef2) into master (1ee0a5c) will decrease coverage by
8.11%
. The diff coverage is0.00%
.
@@ Coverage Diff @@
## master #435 +/- ##
==========================================
- Coverage 73.68% 65.58% -8.11%
==========================================
Files 22 23 +1
Lines 2930 3115 +185
==========================================
- Hits 2159 2043 -116
- Misses 771 1072 +301
Files Changed | Coverage Δ | |
---|---|---|
src/Optimization.jl | 63.50% <0.00%> (-17.81%) |
:arrow_down: |
src/Surrogates.jl | 21.73% <ø> (-17.40%) |
:arrow_down: |
src/VirtualStrategy.jl | 0.00% <0.00%> (ø) |
... and 2 files with indirect coverage changes
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
🎉
As mentioned in #278, it would be nice to be able to be able to have surrogate models ask for multiple points at once to enable parallelized surrogate modeling. This PR introduces an
Ask
function that a user can supply a surrogate model and a set number of points. To maximize the expected improvement of a given model in parallel, temporary virtual values will have to be assigned sequentially for each parallel point. I have implemented 6 different methods of getting virtual points. These areMore information about these virtual point methods can be found in the documentation for the surrogate modeling toolbox and this paper that Scikit Optimize references.
Below is an example of how
Ask
could be used. The functionadd_point
, works as a "tell", so you can have an Ask-tell interface.In its current state, this PR add an
Ask
that only works for Kriging models with an EI acquisition function. I also have not added added documentation yet. I plan to add docs and I would be willing to get this to work for more surrogate model types of acquisition functions, I would just like to get a confirmation that this looks good so far and that I'm on the right track. If there are any suggestions for improvements or ways to other features to add to this PR, I would be happy to hear them.Also mentioned in #278 was that some samplers won't work with parallel sampling. Like it was said there, I think it would make sense to add a type restriction to
Ask
so that only parallelizable samplers work. Is there any way I could add a subtype of the samplers in this repository or would that change have to be made in QuasiMonteCarlo.jl?