Skip to content

Conversation

@y0z
Copy link
Member

@y0z y0z commented Dec 6, 2024

Motivation & Description of the changes

  • Introduce the optunahub.benchmarks module as a new feature for benchmarking.
  • Update the doc files to introduce the module.
image

@y0z y0z added the feature Change that does not break compatibility, but affects the public interfaces. label Dec 6, 2024
@codecov
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.

Project coverage is 88.67%. Comparing base (a3f85ea) to head (acec775).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
optunahub/benchmarks/_base_problem.py 86.36% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
- Coverage   88.80%   88.67%   -0.13%     
==========================================
  Files           6        8       +2     
  Lines         134      159      +25     
==========================================
+ Hits          119      141      +22     
- Misses         15       18       +3     

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

Copy link
Contributor

@nabenabe0928 nabenabe0928 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 the PR, I left some comments!

trial._check_distribution(name, dist)
return self.evaluate(params)

def evaluate(self, params: dict[str, Any]) -> float | Sequence[float]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason why this method is not an abstract method?

Copy link
Member Author

@y0z y0z Dec 11, 2024

Choose a reason for hiding this comment

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

If you only want to support Optuna (e.g., implementing define-by-run objective), it is acceptable to override __call__ and not implement evaluate and search_space.

raise NotImplementedError

@property
def search_space(self) -> dict[str, optuna.distributions.BaseDistribution]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason why this method is not an abstract method?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ditto (cf. #73 (comment))

Copy link
Contributor

@nabenabe0928 nabenabe0928 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 the update, LGTM!

@nabenabe0928 nabenabe0928 merged commit 66a0045 into optuna:main Dec 11, 2024
13 checks passed
@c-bata c-bata added this to the v0.2.0 milestone Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Change that does not break compatibility, but affects the public interfaces.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants