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 95.45455% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.99%. Comparing base (26ee72a) to head (95f1251).

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #549      +/-   ##
==========================================
- Coverage   96.04%   95.99%   -0.06%     
==========================================
  Files          12       12              
  Lines        1139     1149      +10     
==========================================
+ Hits         1094     1103       +9     
- Misses         45       46       +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?

till-m · 2025-04-07T06:22:03Z

bayes_opt/acquisition.py

+
+                # Store it if better than previous minimum(maximum).
+                if min_acq is None or np.squeeze(res.fun) >= min_acq:
+                    x_try_sc = de._unscale_parameters(res.x)


I think this will unscale the result to be between [0, 1], right, which we don't want to do(?) maybe this call to _unscale is not necessary?

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

bayes_opt/acquisition.py

@@ -219,7 +219,7 @@ def _acq_min(
            acq, space, n_random=max(n_random, n_l_bfgs_b), n_x_seeds=n_l_bfgs_b
        )
        if n_l_bfgs_b:
-            x_min_l, min_acq_l = self._l_bfgs_b_minimize(acq, space, x_seeds=x_seeds)
+            x_min_l, min_acq_l = self._smart_minimize(acq, space, x_seeds=x_seeds)


I think the name is good!

till-m · 2025-04-07T06:30:09Z

bayes_opt/acquisition.py

+                    x_try_sc = de._unscale_parameters(res.x)
+                    x_try = space.kernel_transform(x_try_sc).flatten()
+                    x_min = x_try
+                    min_acq = np.squeeze(res.fun)



What do you think about adding a manual polishing step here, i.e. we do one single l-bfgs-b optimization over continuous dimensions, starting from the x_min found by the DE. If I understand correctly, this should work as the polish argument of DE solver does.

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mixed-integer differential evolution for next point suggestion #549

Mixed-integer differential evolution for next point suggestion #549

udicaprio commented Mar 8, 2025 •

edited

Loading

till-m commented Mar 9, 2025

udicaprio commented Mar 9, 2025 •

edited

Loading

till-m commented Mar 9, 2025

codecov bot commented Mar 9, 2025 •

edited

Loading

till-m commented Mar 10, 2025

udicaprio commented Mar 10, 2025

till-m left a comment •

edited

Loading

till-m commented Mar 10, 2025

udicaprio commented Mar 10, 2025

udicaprio commented Mar 10, 2025

till-m commented Mar 11, 2025

till-m commented Mar 13, 2025

udicaprio commented Apr 5, 2025 •

edited

Loading

till-m left a comment

till-m Apr 7, 2025

till-m Apr 7, 2025

till-m Apr 7, 2025

till-m Apr 7, 2025

Mixed-integer differential evolution for next point suggestion #549

Are you sure you want to change the base?

Mixed-integer differential evolution for next point suggestion #549

Conversation

udicaprio commented Mar 8, 2025 • edited Loading

till-m commented Mar 9, 2025

udicaprio commented Mar 9, 2025 • edited Loading

till-m commented Mar 9, 2025

codecov bot commented Mar 9, 2025 • edited Loading

Codecov Report

till-m commented Mar 10, 2025

udicaprio commented Mar 10, 2025

till-m left a comment • edited Loading

Choose a reason for hiding this comment

till-m commented Mar 10, 2025

udicaprio commented Mar 10, 2025

udicaprio commented Mar 10, 2025

till-m commented Mar 11, 2025

till-m commented Mar 13, 2025

udicaprio commented Apr 5, 2025 • edited Loading

till-m left a comment

Choose a reason for hiding this comment

till-m Apr 7, 2025

Choose a reason for hiding this comment

till-m Apr 7, 2025

Choose a reason for hiding this comment

till-m Apr 7, 2025

Choose a reason for hiding this comment

till-m Apr 7, 2025

Choose a reason for hiding this comment

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