feat: Add average parameter to Accuracy for per-label multilabel output#3810
Open
zongyang078 wants to merge 2 commits into
Open
feat: Add average parameter to Accuracy for per-label multilabel output#3810zongyang078 wants to merge 2 commits into
zongyang078 wants to merge 2 commits into
Conversation
Add average parameter to Accuracy, aligning its API with Precision/Recall for multilabel classification. When average=False and is_multilabel=True, compute() returns a per-label accuracy tensor instead of a scalar. Fixes pytorch#513
…ment - cast() the num_correct accumulator before .item() in compute(), matching the pattern Precision already uses for the same int|Tensor duality - drop a comment that just restated the line below it - test_running_average.py: compare _num_correct in a type-agnostic way, since it may now be a plain int (right after reset(), before update()) instead of always a tensor Fixes pytorch#513
| def __init__( | ||
| self, | ||
| output_transform: Callable = lambda x: x, | ||
| average: bool | None = None, |
Collaborator
There was a problem hiding this comment.
we can have it true by default instead of the None
Comment on lines
+320
to
+321
| if average is not None and average is not False: | ||
| raise ValueError("Argument average should be None or False.") |
Collaborator
There was a problem hiding this comment.
Suggested change
| if average is not None and average is not False: | |
| raise ValueError("Argument average should be None or False.") | |
| if type(average) is not bool: | |
| raise ValueError("Argument average should be boolean") |
| @reinit__is_reduced | ||
| def reset(self) -> None: | ||
| self._num_correct = torch.tensor(0, device=self._device) | ||
| self._num_correct: int | torch.Tensor = 0 |
Collaborator
There was a problem hiding this comment.
why its union int tensor?
| y = torch.transpose(y, 1, last_dim - 1).reshape(-1, num_classes) | ||
| if self._average is False: | ||
| correct = (y == y_pred.type_as(y)).float() | ||
| self._num_correct += correct.sum(dim=0).to(self._device) |
Collaborator
There was a problem hiding this comment.
I dont think we need to move it into a new device
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #513
Description:
Add
averageparameter toAccuracyfor multilabel classification, aligning its API withPrecision/Recallwhich already support per-label output via_BasePrecisionRecall.When
average=Falseandis_multilabel=True,compute()returns a per-label accuracy tensor of shape(num_labels,)using elementwise comparison, rather than the default subset accuracy scalar. Default behavior (average=None) is unchanged.Prior attempts (PR #516, PR #542) stalled in 2019 over API design — both proposed a
labelwise=Trueboolean flag, which was rejected. This PR uses theaverageparameter convention already established byPrecision/Recall.Scoped to not touch
Accuracy's internal structure (no_prepare_output), staying orthogonal to #3568 / #3610.Also updates two pre-existing tests (
test_accuracy.py::test_accumulator_device,test_running_average.py::test_integration_batchwise) that assumed_num_correctis always a tensor right afterreset()— it's now a plainintuntil the firstupdate(), matching theint | torch.TensorpatternPrecision/Recallalready use for_numerator/_denominator.Check list: