Skip to content

Add logging with state to Bayesian Optimizer #547

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

Merged
merged 31 commits into from
Mar 17, 2025

Conversation

adrianmolzon
Copy link
Contributor

Hey all, long time listener first time contributor. I've created two new methods for the BayesianOptimizer class to allow users to save and load their optimizers while maintaining state. I've added a new section to the basic tour to demonstrate this cleaner method for saving and loading their optimizer.

A couple notes, users using a custom acquisition function will have to write their own get_acquisition_params and set_acquisition_params class functions in order to preserve exact state, otherwise there might be a loss of state when saving and loading. This should still be strictly better than the previous flow for saving and loading optimizers which ignored the acquisition parameters.

If this pull request gets merged into master, I'd like to remove the previous saving methodology, but I'll save that for a future PR.

@adrianmolzon
Copy link
Contributor Author

@till-m

@till-m
Copy link
Member

till-m commented Mar 9, 2025

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

Copy link
Member

@till-m till-m left a comment

Choose a reason for hiding this comment

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

Generally looks good, especially the extensive testing. I added a few comments here and there.

I think the old save/load functionality should be removed, it's better not to have multiple ways to do the same thing, especially if one of them is clearly superior.

phi-friday and others added 2 commits March 9, 2025 14:49
* chore: upgrade poetry2.0 & apply pep621

* fix: replace poetry action(not support 2.0)

* fix: exclude one matrix

* chore: split numpy deps

* fix: numpy constraints

* fix: install root

* chore: use install-poetry
Copy link

codecov bot commented Mar 9, 2025

Codecov Report

Attention: Patch coverage is 97.91667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.43%. Comparing base (aed6a25) to head (472fd93).
Report is 23 commits behind head on master.

Files with missing lines Patch % Lines
bayes_opt/acquisition.py 96.36% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #547      +/-   ##
==========================================
+ Coverage   96.19%   96.43%   +0.24%     
==========================================
  Files          10       12       +2     
  Lines         867     1235     +368     
==========================================
+ Hits          834     1191     +357     
- Misses         33       44      +11     

☔ 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.

@till-m
Copy link
Member

till-m commented Mar 12, 2025

I think this is ready to go, I will merge this in the coming days if there are no objections.

@till-m till-m merged commit 626aafe into bayesian-optimization:master Mar 17, 2025
14 checks passed
@till-m
Copy link
Member

till-m commented Mar 17, 2025

Thanks for your contribution!

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.

3 participants