Mixed-integer differential evolution for next point suggestion #549

udicaprio · 2025-03-08T16:27:01Z

Hi,

this commit solves the pull request #548. It implements mixed-integer differential evolution (MIP-DE) to minimise the acquisition function in case of a mixed-integer optimisation.
The MIP-DE is implemented only in case discrete variables are present in the problem.

Code linter optimisation with Ruff

till-m · 2025-03-09T07:29:18Z

@udicaprio I realized the coverage bot is broken, so let me fix that first before moving on with this PR

udicaprio · 2025-03-09T07:43:35Z

@till-m, fine no problem. I just realized that I could make some easy modification that improve the computational performance without affecting the identifications one that I would like to implement.

I will be working on this today, it is a quite an easy one but I need to fine-tune some DE parameters.

till-m · 2025-03-09T08:02:51Z

Great! Could you tag me or request a review when you're done?
I think the coverage is fixed. Could you pull from upstream? Then, it should work for this PR as well :)

codecov · 2025-03-09T09:15:15Z

Codecov Report

Attention: Patch coverage is 96.87500% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.87%. Comparing base (1714504) to head (79b8e03).
Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
bayes_opt/acquisition.py	96.87%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #549      +/-   ##
==========================================
- Coverage   97.92%   97.87%   -0.06%     
==========================================
  Files          10       10              
  Lines        1158     1175      +17     
==========================================
+ Hits         1134     1150      +16     
- Misses         24       25       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Computational-efficiency of the MIP identification increased

Linter improved

till-m · 2025-03-10T11:18:28Z

FYI I need to manually approve the tests since you're a first-time contributor. If I forget to run them, please tag me and I will make sure they run. Also, don't worry about small changes in coverage. We're trying to cover the code as much as makes sense, not chase a number.

udicaprio · 2025-03-10T13:03:29Z

Hello @till-m, thank you for the notification. Seems the code passed all the tests except the coverage one.
Can I consider it ready for merging, or should I make any further modifications in order to reach at least the old coverage percentage?

till-m

Generally speaking, this is great code: Clear, concise, etc. I've left some minor comments, though some are just to clarify things. One big issue, however, is assuming that any non-continuous parameter is necessarily integer valued... I don't see any easy way around this right now, though I assume that the algorithm isn't inherently incompatible with discrete, non-int parameters.

should I make any further modifications in order to reach at least the old coverage percentage?

The line that is currently not covered is perfectly fine to not cover, imo.

bayes_opt/acquisition.py

till-m · 2025-03-10T13:37:33Z

Ah, also, the name of the function should probably be adjusted somewhat. I'm not sure what the best name would be -- something like _structured_minimize? _deterministic_minimize (c.f. the _random_sample_minimize, though since DE is somewhat random this isn't entirely correct)? _smart_minimize? _l_bfgs_b_or_de_minimize?

udicaprio · 2025-03-10T18:40:27Z

Ah, also, the name of the function should probably be adjusted somewhat. I'm not sure what the best name would be -- something like _structured_minimize? _deterministic_minimize (c.f. the _random_sample_minimize, though since DE is somewhat random this isn't entirely correct)? _smart_minimize? _l_bfgs_b_or_de_minimize?

Working on this

udicaprio · 2025-03-10T18:57:34Z

@till-m, thank you for revising the code and your comments. I am working on them. I will push the modifications as soon as possible.

till-m · 2025-03-11T09:59:50Z

Excellent, thank you, much appreciated 🙏

till-m · 2025-03-13T08:51:51Z

@udicaprio

Regarding the discrete != integrable discussion, I know that this significantly complicates the PR and it might not be what you've signed up for when you offered to implement DE optimization. If there's something I can do to make things easier for you, let me know. I'm convinced that this is an extremely valuable contribution to the library so I'd like to make sure it gets added!

udicaprio · 2025-04-05T10:29:48Z

Hello @till-m, I have modified the DE search including the transformation with the kernel_transform method of the space. Please let me know if this is fine or any further modification is required.

till-m

Hey @udicaprio, things have been/are a bit busy on my end, sorry about that. I think this is almost there, but I left a few small comments that should be addressed, I think.

Thanks for all the work!

till-m · 2025-04-07T06:20:04Z

bayes_opt/acquisition.py

+            ntrials = max(1, len(x_seeds) // 100)
+            for _ in range(ntrials):
+                xinit = space.random_sample(15 * len(space.bounds), random_state=self.random_state)
+                de = DifferentialEvolutionSolver(acq, bounds=space.bounds, init=xinit, rng=self.random_state)


I think we need to use polish=False to turn off gradient-based polishing?

bayes_opt/acquisition.py

…continuous parameter search

…version compatibility

udicaprio · 2025-04-27T14:30:16Z

Hello @till-m, I have just completed the code revision with your comments. Additionally, I have added a version check on Scipy and reformatting of the differential evolution input since the one about the random seed has been changed at version 1.15. Can you please let the workflow run?

till-m

Looking great, I think! I have one small comment regarding the code. Other than that I think the final thing I would ask you to do is the check the references within the code to l-bfgs-b. I think in many cases variable names and docstrings should change to reflect that we're not exclusively doing L-BFGS-B Optimization anymore. After this, it should be good to merge.

bayes_opt/acquisition.py

till-m · 2025-04-28T08:38:46Z

Sidenote: Do you think we need to make sure that we test on scipy over/under 1.15.0?

…lies kernel transformation to x_min

udicaprio · 2025-05-01T06:45:27Z

Hello @till-m, I applied the suggested modification. Can you please trigger the testing pipeline?
About the tests for scipy over/under 1.15.0, I am not sure this is needed. I realised of the error because of a faulty test following the pushing on the pull request. Therefore I guess the existing testing routines inherently tests for it (at least at an API level).

udicaprio · 2025-05-01T07:36:19Z

Hello @till-m, I forgot to check the linter. Should be fixed now. Can you please re-trigger the test?

till-m · 2025-05-02T08:56:28Z

Sorry, I missed that message. Rerunning now!

hjmjohnson · 2025-05-04T13:30:04Z

@udicaprio This is a spectacular enhancement to the library. Thank you for your efforts to implement this vital feature.

till-m

LGTM 🚀 will merge soon! :)

udicaprio · 2025-05-06T16:51:55Z

@udicaprio This is a spectacular enhancement to the library. Thank you for your efforts to implement this vital feature.

Thank you very much

udicaprio · 2025-05-06T16:52:14Z

LGTM 🚀 will merge soon! :)

Thank you! Looking forward to other contributions

till-m · 2025-05-07T08:39:28Z

Thanks for your contribution! :)

Update acquisition.py

d12fd04

udicaprio changed the title ~~Update acquisition.py~~ Mixed-integer differential evolution for next point suggestion Mar 8, 2025

Update acquisition.py

57e1a4d

Code linter optimisation with Ruff

Merge branch 'bayesian-optimization:master' into master

bfb98ca

udicaprio added 3 commits March 9, 2025 09:46

Update acquisition.py

0cd9176

Computational-efficiency of the MIP identification increased

Update acquisition.py

380fe64

Linter improved

Update acquisition.py

95f1251

till-m reviewed Mar 10, 2025

View reviewed changes

bayes_opt/acquisition.py Outdated Show resolved Hide resolved

bayes_opt/acquisition.py Outdated Show resolved Hide resolved

bayes_opt/acquisition.py Show resolved Hide resolved

bayes_opt/acquisition.py Outdated Show resolved Hide resolved

udicaprio added 2 commits April 5, 2025 09:57

Rename minimization methods for clarity and update tests accordingly

fb101a6

Refactor differential evolution import and optimize acquisition function

0e859df

till-m reviewed Apr 7, 2025

View reviewed changes

udicaprio and others added 3 commits April 26, 2025 10:55

Merge branch 'bayesian-optimization:master' into master

0b6148f

Modification to differential evolution solver integration and refine …

fb9d6a2

…continuous parameter search

Refactor DifferentialEvolutionSolver initialization to support scipy …

07f75b2

…version compatibility

till-m reviewed Apr 28, 2025

View reviewed changes

bayes_opt/acquisition.py Show resolved Hide resolved

Removes fixed algorithm for acquisition function minimization and app…

a241f62

…lies kernel transformation to x_min

Fixing formatting issue

79b8e03

till-m approved these changes May 5, 2025

View reviewed changes

till-m merged commit c59cb64 into bayesian-optimization:master May 7, 2025
12 of 14 checks passed

Mixed-integer differential evolution for next point suggestion #549

Mixed-integer differential evolution for next point suggestion #549

Uh oh!

Conversation

udicaprio commented Mar 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

till-m commented Mar 9, 2025

Uh oh!

udicaprio commented Mar 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

till-m commented Mar 9, 2025

Uh oh!

codecov bot commented Mar 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

till-m commented Mar 10, 2025

Uh oh!

udicaprio commented Mar 10, 2025

Uh oh!

till-m left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

till-m commented Mar 10, 2025

Uh oh!

udicaprio commented Mar 10, 2025

Uh oh!

udicaprio commented Mar 10, 2025

Uh oh!

till-m commented Mar 11, 2025

Uh oh!

till-m commented Mar 13, 2025

Uh oh!

udicaprio commented Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

till-m left a comment

Choose a reason for hiding this comment

Uh oh!

till-m Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

udicaprio commented Apr 27, 2025

Uh oh!

till-m left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

till-m commented Apr 28, 2025

Uh oh!

udicaprio commented May 1, 2025

Uh oh!

udicaprio commented May 1, 2025

Uh oh!

till-m commented May 2, 2025

Uh oh!

hjmjohnson commented May 4, 2025

Uh oh!

till-m left a comment

Choose a reason for hiding this comment

Uh oh!

udicaprio commented May 6, 2025

Uh oh!

udicaprio commented May 6, 2025

Uh oh!

Uh oh!

till-m commented May 7, 2025

Uh oh!

Uh oh!

udicaprio commented Mar 8, 2025 •

edited

Loading

udicaprio commented Mar 9, 2025 •

edited

Loading

codecov bot commented Mar 9, 2025 •

edited

Loading

till-m left a comment •

edited

Loading

udicaprio commented Apr 5, 2025 •

edited

Loading