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

🎩 🎸 Class-Based Rank-Based Metrics #786

Merged
merged 119 commits into from
Mar 4, 2022
Merged

🎩 🎸 Class-Based Rank-Based Metrics #786

merged 119 commits into from
Mar 4, 2022

Conversation

mberr
Copy link
Member

@mberr mberr commented Feb 16, 2022

Part 7(?) of the #744 destruction train

It also changes the canonical order of metrics from metric_name.side.rank_type.k? to side.rank_type.metric_name.k?

Tasks

tests/cases.py Outdated Show resolved Hide resolved
@cthoyt
Copy link
Member

cthoyt commented Feb 21, 2022

So at this point, this PR now defines all of the ranking metrics through classes but only applies a subset of them this way. To finish the job, this PR will have to get quite a bit more complicated. What do you think @mberr? I'm not sure that there are any more meaningful checkpoints between what's already on master and what it will take to finish the job, so maybe we might want to just bite the bullet and rewrite the evaluator's part that handles these

@mberr
Copy link
Member Author

mberr commented Feb 21, 2022

So at this point, this PR now defines all of the ranking metrics through classes but only applies a subset of them this way. To finish the job, this PR will have to get quite a bit more complicated. What do you think @mberr? I'm not sure that there are any more meaningful checkpoints between what's already on master and what it will take to finish the job, so maybe we might want to just bite the bullet and rewrite the evaluator's part that handles these

agreed, let's do this in one PR.

@cthoyt cthoyt changed the title Class-Based Metrics 🎩 🎸 Class-Based Rank-Based Metrics Feb 21, 2022
trigger ci
trigger ci
@mberr mberr marked this pull request as ready for review March 4, 2022 14:42
@mberr mberr mentioned this pull request Mar 4, 2022
1 task
@mberr mberr requested a review from cthoyt March 4, 2022 18:30
@cthoyt cthoyt enabled auto-merge (squash) March 4, 2022 20:48
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

2 participants