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

Parallel Surrogate Models Through Ask-tell Interface #435

Merged
merged 16 commits into from
Sep 22, 2023

Conversation

dreycenfoiles
Copy link
Contributor

@dreycenfoiles dreycenfoiles commented Jun 11, 2023

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 are

  • Minimum Constant Liar: virtual points are chosen based on the smallest known value of the provided surrogate
  • Mean Constant Liar: virtual points are chosen based on the smallest known value of the provided surrogate
  • Maximum Constant Liar: virtual points are chosen based on the smallest known value of the provided surrogate
  • Kriging Believer: virtual points are chosen using the value of the current Kriging model
  • Kriging Believer Upper bound: virtual points are chosen using the $3\sigma$ upper bound of the current Kriging model
  • Kriging Believer Lower bound: virtual points are chosen using the $3\sigma$ lower bound of the current Kriging model

More 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 function add_point, works as a "tell", so you can have an Ask-tell interface.

using Surrogates

lb = 0.0
ub = 10.0
f = x -> log(x) * exp(x)
x = sample(5, lb, ub, SobolSample())
y = f.(x)

my_k = Kriging(x, y, lb, ub)

for _ in 1:10
    new_x, eis = Ask(EI(), my_k, SobolSample(), 3, CLmean!)
    add_point!(my_k, new_x, f.(new_x))
end

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?

@ChrisRackauckas
Copy link
Member

Hey, this is really cool and it's sad to see it just live as a draft. Is there an intention to complete this?

@dreycenfoiles
Copy link
Contributor Author

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.

@dreycenfoiles dreycenfoiles marked this pull request as ready for review September 17, 2023 03:12
@dreycenfoiles
Copy link
Contributor Author

dreycenfoiles commented Sep 17, 2023

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.

dtol = 1e-3 * norm(ub - lb)
eps = 0.01

tmp_krig = deepcopy(krig) # Temporary copy of the kriging model to store virtual points
Copy link
Member

Choose a reason for hiding this comment

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

This can be pretty heavy. Did you check it did not cause a significant performance issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did some tests and it seems like in all cases, deepcopy will never be anywhere close to your bottleneck. The bottleneck is always either evaluating your objective function or calculating the expected improvement or merit function.

@ChrisRackauckas
Copy link
Member

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.

@dreycenfoiles
Copy link
Contributor Author

I think I was able to address your concerns. Is there anything else you'd like me to change?

my_k = Kriging(x, y, lb, ub)

for _ in 1:10
new_x, eis = potential_optimal_points(EI(), lb, ub, my_k, SobolSample(), 3, CLmean!)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
new_x, eis = potential_optimal_points(EI(), lb, ub, my_k, SobolSample(), 3, CLmean!)
new_x, eis = potential_optimal_points(EI(), lb, ub, my_k, SobolSample(), 3, MeanConstantLiar())

?

@ChrisRackauckas
Copy link
Member

I think this looks great! It may need to wait for #441 to fix tests on master and rebase on top of that, but I think this is looking ready to merge if tests pass.

@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Merging #435 (ea4eef2) into master (1ee0a5c) will decrease coverage by 8.11%.
The diff coverage is 0.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%) ⬇️
src/Surrogates.jl 21.73% <ø> (-17.40%) ⬇️
src/VirtualStrategy.jl 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ChrisRackauckas ChrisRackauckas merged commit 9182073 into SciML:master Sep 22, 2023
3 of 7 checks passed
@ChrisRackauckas
Copy link
Member

🎉

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.

2 participants