Skip to content

Refactor evaluation #96

Merged
alessiodevoto merged 32 commits intomainfrom
aledev/eval
Jul 9, 2025
Merged

Refactor evaluation #96
alessiodevoto merged 32 commits intomainfrom
aledev/eval

Conversation

@alessiodevoto
Copy link
Contributor

@alessiodevoto alessiodevoto commented Jul 9, 2025

PR description

Description of your PR. Fixes # (issue) (if applicable)

Checklist

  • Tests are working (make test)
  • Code is formatted correctly (make style, on errors try fix with make format)
  • Copyright header is included
  • All commits are signed-off using git commit -s
  • (new press) mypress_press.py is in the presses directory
  • (new press) MyPress is in __init__.py
  • (new press) README.md is updated with a 1 liner about the new press in the Available presses section
  • (new press) New press is in the default_presses list in tests/default_presses.py
  • (new press) A docstring is provided that follows the same structure as the existing ones

Details

Addresses #89 with the following changes/tests:

  • Moved all datasets to the benchmarks dir to prevent clutter (all files left unchanged)
  • Refactored the code for evaluation to work both with yaml file and CLI args (see evaluation.py)
  • Added instructions to README
  • Tested all presses on a single sample from ruler

Additional change

  • minimal change to QFilterPress that was not loading q_filter files correctly due to a mismatch in model names (Meta-Llama -> Llama)

maxjeblick and others added 29 commits July 3, 2025 12:38
@maxjeblick
claude 4 generated docstrings
d2c31a3 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
shorter docstrings
08a43bf 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
shorter docstrings
5c6d1c5 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
update readme
113fbae 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
update readme
4673b96 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
update readme
b0a8f70 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
update readme
adca7bb 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
update search_hyperplane doc
6748cdb 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
update search_hyperplane doc
d51b959 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
update search_hyperplane doc
7a0599b 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
improve referecnes
3933075 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
address PR feedback
487b9a4 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
address PR feedback
2a71f26 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
address PR feedback
bbf2454 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@maxjeblick
edits on readme
d5c701d 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@alessiodevoto
refactor eval
0840de2 
Signed-off-by: alessiodevoto <adevoto@nvidia.com>
@maxjeblick
Merge branch 'main' into max/improve_docs
d58af5e
@maxjeblick
update version
bf52155 
Signed-off-by: Maximilian Jeblick <maximilianjeblick@gmail.com>
@alessiodevoto
update docs + filename
95f79fe 
Signed-off-by: alessiodevoto <adevoto@nvidia.com>
@alessiodevoto
updates from hotfix
e24a19f 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
allow model kwargs + refactor datasets
428db6e 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
merge main
7b6b2c5 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
update readme
c6addbf 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
style
5dcd319 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
update qfilter
c1d6324 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
fix isintance + qfilt
60ab114 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
import order
58edd2a 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
update readme
92b6642 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
fix style
79ac20c 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
maxjeblick
maxjeblick requested changes Jul 9, 2025
View reviewed changes
Copy link
Collaborator

@maxjeblick maxjeblick left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR, overall, it looks really good!
I have two suggestions:

  1. Create a dedicated Config class instead of using a dictionary. This could improve readability a bit.
  2. Instead of encoding the config settings in the dataframe/metrics name, create a folder that also saves the config. By this, one can easily load the config.yaml again for downstream processing.

Item 1. is a potential refactoring; 2. should probably be implemented, as the current implementation is a bit hacky.

@alessiodevoto
use dataclass
a48a3e5 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
Copy link
Contributor Author

alessiodevoto commented Jul 9, 2025

Thanks, the dataclass is way better indeed! I implemented it and changed the output directory pattern. Now it is like:

<dir name from configs>/
    ├── predictions.csv
    ├── metrics.json
    └── eval_config.yaml

@alessiodevoto
Copy link
Contributor Author

alessiodevoto commented Jul 9, 2025

I put the dataclass code in the evaluation.py file, maybe we can move it somewhere else (just wanted to avoid too many files)

maxjeblick
maxjeblick approved these changes Jul 9, 2025
View reviewed changes
Copy link
Collaborator

@maxjeblick maxjeblick left a comment

Choose a reason for hiding this comment

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

Thanks a lot LGTM!

@alessiodevoto
default cr
f2b5097 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
@alessiodevoto
style
771f0f8 
Signed-off-by: alessiodevoto <devoto.alessio@gmail.com>
maxjeblick
maxjeblick approved these changes Jul 9, 2025
View reviewed changes
Copy link
Collaborator

@maxjeblick maxjeblick left a comment

Choose a reason for hiding this comment

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

Thanks a lot LGTM!

@alessiodevoto alessiodevoto merged commit 4e559d3 into main Jul 9, 2025
3 checks passed
@alessiodevoto alessiodevoto deleted the aledev/eval branch July 9, 2025 15:03
maxjeblick pushed a commit that referenced this pull request Aug 12, 2025
@alessiodevoto @maxjeblick
Refactor evaluation (#96)
f9fcc5d 
Signed-off-by: Max Jeblick <maximilianjeblick@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maxjeblick maxjeblick maxjeblick approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alessiodevoto @maxjeblick