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.

Sign up for GitHub

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

Jump to bottom

Refactor benchmark architecture #477

Merged
merged 22 commits into from
Feb 7, 2025
Merged

Refactor benchmark architecture #477

merged 22 commits into from
Feb 7, 2025

Conversation

AdrianSosic
Copy link
Collaborator

This PR refines the benchmarking classes and fixes a few other things along the way:

  • Most importantly, the convergence-test-specific optimal_function_inputs and best_possible_result attributes are moved to a separate ConvergenceBenchmark class, with corresponding ConvergenceBenchmarkSettings.
  • The remaining benchmark attributes / properties are cleaned up.

@AdrianSosic AdrianSosic self-assigned this Feb 5, 2025
@AdrianSosic AdrianSosic marked this pull request as ready for review February 5, 2025 21:09
AdrianSosic
AdrianSosic commented Feb 5, 2025
View reviewed changes
benchmarks/definition/convergence.py Outdated Show resolved Hide resolved
benchmarks/definition/base.py Show resolved Hide resolved
AdrianSosic and others added 18 commits February 7, 2025 14:19
@AdrianSosic
Rename ConvergenceExperimentSettings to ConvergenceBenchmarkSettings
fe99d1f
@AdrianSosic
Extract convergence-related attributes into subclass
4f4abcd
@AdrianSosic
Move convergence benchmark functionality to separate module
bba2c77
@AdrianSosic
Instantiate settings type
3618972
@AdrianSosic
Make settings classes kw-only
41a1163
@AdrianSosic
Rename config module to base
7fc50cc
@AdrianSosic
Reorder benchmark attributes
f552a94
@AdrianSosic
Ensure that the benchmark function has a docstring
d6c7ef0
@AdrianSosic
Drop unused name attribute
64e1a17
@AdrianSosic
Switch to modern cattrs hook registration mechanism
8c4cb6d
@AdrianSosic
Update lockfile
9afcb1b
@AdrianSosic
Refine some docstrings
689ec00
@fabianliebig @AdrianSosic
Update branch attribute to allow None and filter out None components …
c8fd731 
…in path construction
@AdrianSosic
Simplify validator using built-in optional decorator
828c804
@AdrianSosic
Fix branch attribute for detached head cases
acb82b1
@AdrianSosic
Refactor ConvergenceBenchmark attributes
6c0eaff 
* Drop optimal_function_inputs because it's not needed
* Rename and refactor best_possible_result so that it contains the
  optima for all targets individually
@AdrianSosic
Correctly unstructure Benchmark subclasses
29fd0d7
@AdrianSosic
Bring back benchmark name as property
cf88fe2 
Should not have been removed since used as identifier for Result,
but implementation as property makes more sense.
@AdrianSosic AdrianSosic merged commit ae64ca1 into main Feb 7, 2025
6 of 11 checks passed
@AdrianSosic AdrianSosic deleted the refactor/benchmarks branch February 7, 2025 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabianliebig fabianliebig fabianliebig approved these changes

@Scienfitz Scienfitz Scienfitz approved these changes

@AVHopp AVHopp Awaiting requested review from AVHopp AVHopp is a code owner

Assignees

@AdrianSosic AdrianSosic

Labels
refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AdrianSosic @Scienfitz @fabianliebig