feat: Add average parameter to Accuracy for per-label multilabel output - #3810
feat: Add average parameter to Accuracy for per-label multilabel output#3810zongyang078 wants to merge 3 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, |
There was a problem hiding this comment.
we can have it true by default instead of the None
There was a problem hiding this comment.
Makes sense. Switching to average: bool = True with True as the default keeps it simple. But I want to flag a naming conflict before changing the default. In Precision/Recall, average=True means "macro average," not "current default behavior." Using True to mean "subset accuracy" here would be semantically different from the sibling classes. Happy to go either way — do you think the simplicity of a plain bool outweighs the consistency concern, or should we keep None to leave room for future 'micro'/'macro' modes?
There was a problem hiding this comment.
Making it None doesn't make is consistent either if consistency is the problem rename it possibly
| if average is not None and average is not False: | ||
| raise ValueError("Argument average should be None or False.") |
There was a problem hiding this comment.
| 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") |
There was a problem hiding this comment.
Will update if we go with the bool-only approach from the comment above.
| 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) |
There was a problem hiding this comment.
I dont think we need to move it into a new device
There was a problem hiding this comment.
I kept it for consistency with the existing default path a few lines below (self._num_correct += torch.sum(correct).to(self._device)), which does the same thing. Happy to remove both if you'd prefer, or keep both — just want them consistent.
There was a problem hiding this comment.
Its redundant in my opinion
2 participants
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: