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

Add parameters as columns in result file #703

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Melvin-klein
Copy link
Member

Checks before merging PR

  • added documentation for any new feature
  • added unit test
  • edited the what's new (if applicable)

@Melvin-klein Melvin-klein marked this pull request as draft April 24, 2024 07:28
benchopt/runner.py Outdated Show resolved Hide resolved
@Melvin-klein Melvin-klein marked this pull request as ready for review April 24, 2024 12:31
@codecov-commenter
Copy link

Codecov Report

Merging #703 (e88520b) into main (e3a3b0d) will decrease coverage by 0.04%.
The diff coverage is 46.15%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #703      +/-   ##
==========================================
- Coverage   54.30%   54.27%   -0.04%     
==========================================
  Files          45       45              
  Lines        2994     3007      +13     
  Branches      560      565       +5     
==========================================
+ Hits         1626     1632       +6     
- Misses       1243     1249       +6     
- Partials      125      126       +1     

if key in slv_parameters:
slv_parameters["solver_" + key] = slv_parameters.pop(key)
if key in ds_parameters:
ds_parameters["dataset_" + key] = ds_parameters.pop(key)
Copy link
Member

Choose a reason for hiding this comment

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

Thinking back after our discussion, this solution is not working :(
If you have two solvers: solver1 with param a and solver2 with no param, and a dataset with param a, you will end up with 3 columns for param a, depending on the combination.

I think the best course of action is to use prefix all the time. It is a little bit ugly but it is robust and don't require too much dev compared to introspecting everything.

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.

None yet

3 participants