-
Notifications
You must be signed in to change notification settings - Fork 406
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
fix: correct unification implementation for RankingQuestionStrategy
#4295
Conversation
The URL of the deployed environment for this PR is https://argilla-quickstart-pr-4295-ki24f765kq-no.a.run.app |
Hi @davidberenstein1957, I've removed the # representation as a dataframe of the rankings
>>> df
value rank
0 (yes, no) (2, 3)
1 (yes, no) (2, 1)
2 (yes, no) (2, 3) The mean in this case would be (asumming we want the mean in the ranks, and obtain the mean of each "row" in the rank): 0 (yes, no) (2, 3)
2 (yes, no) (2, 3) I think that it's a bit hard to reason of a mean for the rankings, when we would aim to obtain the majority (or mode) in a sense, what do you think? I can correct the tests that assume the mean strategy exists for the |
@@ -202,13 +204,11 @@ def unify_responses(self, records: List[FeedbackRecord], question: str): | |||
class RankingQuestionStrategy(RatingQuestionStrategyMixin, Enum): | |||
""" | |||
Options: | |||
- "mean": the mean value of the rankings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def calculate_average_ranking(data):
label_rank_sum = {}
label_count = {}
for ranking in data:
for item in ranking:
label, rank = item.popitem()
label_rank_sum[label] = label_rank_sum.get(label, 0) + rank
label_count[label] = label_count.get(label, 0) + 1
average_ranking = {label: label_rank_sum[label] / label_count[label] for label in label_rank_sum}
return average_ranking
# Example usage:
data = [[{"label_1": 2}, {"label_2": 1}], [{"label_1": 1}, {"label_2": 2}]]
result = calculate_average_ranking(data)
print(result)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
after this I would only expect the labels to be mapped back to the original available ranks through an zip(rank, sorted_result_based_on_values)
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #4295 +/- ##
===========================================
- Coverage 64.76% 64.66% -0.10%
===========================================
Files 321 321
Lines 18511 18540 +29
===========================================
+ Hits 11988 11989 +1
- Misses 6523 6551 +28 ☔ View full report in Codecov by Sentry. |
While testing more functionality for the metrics I've found another bug, will try to fix it using this PR: Testing with the following dataset from huggingface (plaguss/go_emotions_raw) >>> feedback_dataset.records[0].responses
[ResponseSchema(user_id=UUID('00000000-0000-0000-0000-000000000001'), values={'label': ValueSchema(value=['neutral'])}, status=<ResponseStatus.submitted: 'submitted'>),
ResponseSchema(user_id=UUID('00000000-0000-0000-0000-000000000016'), values={'label': ValueSchema(value=['anger', 'annoyance', 'optimism'])}, status=<ResponseStatus.submitted: 'submitted'>),
ResponseSchema(user_id=UUID('00000000-0000-0000-0000-000000000028'), values={'label': ValueSchema(value=['approval'])}, status=<ResponseStatus.submitted: 'submitted'>),
ResponseSchema(user_id=UUID('00000000-0000-0000-0000-000000000039'), values={'label': ValueSchema(value=['neutral'])}, status=<ResponseStatus.submitted: 'submitted'>),
ResponseSchema(user_id=UUID('00000000-0000-0000-0000-000000000048'), values={'label': ValueSchema(value=['annoyance'])}, status=<ResponseStatus.submitted: 'submitted'>)] And after unifying the responses: feedback_dataset.records[0].unified_responses
{'label': [UnifiedValueSchema(value=[], strategy=<RatingQuestionStrategy.MAJORITY: 'majority'>)]} We should have one of the labels contained in the responses. Solved in the following commit. |
* develop: (30 commits) chore: increase dev version release to 1.21.0 fix: responses and suggestions filter QA (#4337) feat: delete suggestion from record on search engine (#4336) feat: update suggestion from record on search engine (#4339) bug: fix bug and update test (#4341) fix: preserve `TextClassificationSettings.label_schema` order (#4332) Update issue templates feat: 🚀 support for filtering and sorting by responses and suggestions (#4160) fix: handling errors for non-existing endpoints (#4325) feat: adding utils module and functions (#4121) Update labels in github workflows (#4315) fix: correct unification implementation for `RankingQuestionStrategy` (#4295) fix: update to solve the error of integration tests in CI (#4314) docs: revisit install process (#4261) feat: increase timeout minutes for python tests (#4307) docs: docs export dataset does not apply coloring for code snippets (#4296) docs: update final section of the rag haystack blog post (#4294) feat: add multi_modal templates and update vector setting (#4283) feat: better logging bar for FeedbackDataset (#4267) refactor: ArgillaTrainer for unified variable usage (#4214) ... # Conflicts: # frontend/v1/infrastructure/repositories/RecordRepository.ts
Description
Currently we have the following behaviour for :
Where we should have:
This PR fixes the issue
Type of change
(Please delete options that are not relevant. Remember to title the PR according to the type of change)
How Has This Been Tested
(Please describe the tests that you ran to verify your changes. And ideally, reference
tests
)tests/integration/client/feedback/test_unification.py
Checklist
CHANGELOG.md
file (See https://keepachangelog.com/)