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

Introduce SimpleBaseSampler and remove load settings propagation #55

Merged
merged 6 commits into from
Aug 23, 2024

Conversation

y0z
Copy link
Member

@y0z y0z commented Aug 22, 2024

Motivation

We've decided to move SimpleBaseSampler from optunahub-registry to optunahub.samplers and remove settings propagation. This PR introduces SimpleBaseSampler and removes things that are no longer needed.

Description of the changes

  • Add optunahub.samplers.SimpleBaseSampler
  • Add docs for optunahub.samplers.SimpleBaseSampler
  • Remove code for settings propagation from load_module and load_local_module
  • Remove tests for settings propagation

Copy link

codecov bot commented Aug 22, 2024

Codecov Report

Attention: Patch coverage is 88.37209% with 5 lines in your changes missing coverage. Please review.

Project coverage is 81.95%. Comparing base (5cc1871) to head (59fedf3).
Report is 17 commits behind head on main.

Files Patch % Lines
optunahub/samplers/_simple_base.py 87.17% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #55      +/-   ##
==========================================
+ Coverage   81.90%   81.95%   +0.05%     
==========================================
  Files           4        6       +2     
  Lines         105      133      +28     
==========================================
+ Hits           86      109      +23     
- Misses         19       24       +5     

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

Copy link
Member

@toshihikoyanase toshihikoyanase left a comment

Choose a reason for hiding this comment

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

Thank you for your PR. The change looks good to me.

I confirmed that optunahub.samplers.SimpleBaseSampler actually worked with the whale optimization.

Since this PR includes some breaking changes, it might be kind to cover the changes regarding propagation in the PR title.



class SimpleBaseSampler(BaseSampler, abc.ABC):
"""A simple base class to implement user-defined samplers."""
Copy link
Member

Choose a reason for hiding this comment

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

[Note] This implementation is a copy of https://github.com/optuna/optunahub-registry/blob/main/package/samplers/simple/__init__.py, except for this docstring.

@y0z y0z changed the title Introduce SimpleBaseSampler Introduce SimpleBaseSampler and remove load settings propagation Aug 23, 2024
@toshihikoyanase toshihikoyanase merged commit e55a73d into optuna:main Aug 23, 2024
13 checks passed
@y0z y0z deleted the feature/simple-base-sampler branch August 23, 2024 04:18
@y0z y0z added the compatibility Change that breaks compatibility. label Aug 28, 2024
@y0z y0z added this to the v0.1.0 milestone Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility Change that breaks compatibility.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants