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

Remove label keys from lite classification #776

Merged
merged 4 commits into from
Oct 4, 2024

Conversation

ntlind
Copy link
Contributor

@ntlind ntlind commented Oct 4, 2024

Improvements

  • Update lite tests and functions to no longer use label keys

@ntlind ntlind requested review from czaloom and ekorman as code owners October 4, 2024 19:11
@ntlind ntlind self-assigned this Oct 4, 2024
@ntlind ntlind added the enhancement New feature or request label Oct 4, 2024
@@ -173,35 +171,24 @@ def classifications_image_example() -> list[Classification]:
Classification(
uid="uid5",
groundtruths=[
("k4", "v4"),
("k5", "v5"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cut the "k5" and "k3" entries out of this data, rather than replicating our tests 3x everywhere this data is used, because these keys didn't seem worth repeating as separate, unique tests: they both just test for a false negative / false positive combo, which is already tested in "uid5" by gt=['v4'] and pd=['v1', 'v8']

{
"type": "Accuracy",
"value": [0.0],
"value": [0.5], # TODO add comment about the bug here
Copy link
Contributor Author

@ntlind ntlind Oct 4, 2024

Choose a reason for hiding this comment

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

this value used to be .333, which (I believe) was a bug: if you look at test_evaluate_image_clf in integration_tests/..., this value should have been .5. no action needed now that label keys are removed, just explaining why this changed

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great catch! The 1/3 accuracy is b/c we are dividing by n_datums in the computation. This resulted in a flawed output since that label key didnt exist for one datum.

@ntlind ntlind merged commit 774ca78 into main Oct 4, 2024
16 checks passed
@ntlind ntlind deleted the remove_label_keys_from_classification branch October 4, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants