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.

Sign up for GitHub

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

Make _stable_1d_sort(nb) optional #196

Closed
wants to merge 2 commits into from
Closed

Make _stable_1d_sort(nb) optional #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e873d9
Make `_stable_1d_sort(nb)` optional
carmocca Apr 22, 2021
25c8a2d
Import and typing
carmocca Apr 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions tests/classification/test_auc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@

import numpy as np
import pytest
import torch
from sklearn.metrics import auc as _sk_auc
from torch import tensor

from tests.helpers import seed_all
from tests.helpers.testers import NUM_BATCHES, MetricTester
from torchmetrics.classification.auc import AUC
from torchmetrics.functional import auc
from torchmetrics.utilities.data import _stable_1d_sort

seed_all(42)

Expand Down Expand Up @@ -76,3 +78,12 @@ def test_auc_functional(self, x, y):
def test_auc(x, y, expected):
# Test Area Under Curve (AUC) computation
assert auc(tensor(x), tensor(y), reorder=True) == expected


@pytest.mark.parametrize("nb", (None, 5, 15))
def test_stable_1d_sort(nb):
Copy link
Contributor

Choose a reason for hiding this comment

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

in your test, you are actually not testing stability at all, you are just testing if the sort is working at all. Can you try seeing if sorting an array of 8 zeros (and other equal values) produces indices=[0,1,2,3,..7] (so no changes are made by sort, meaning it's stable)

n = 10
x = torch.arange(n)
x_shuf = torch.randperm(n)
y, _ = _stable_1d_sort(x_shuf, nb=nb)
assert torch.equal(x[:nb], y)
15 changes: 8 additions & 7 deletions torchmetrics/utilities/data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from typing import Any, Callable, List, Mapping, Optional, Sequence, Union
from typing import Any, Callable, List, Mapping, Optional, Sequence, Tuple, Union

import numpy as np
import torch
Expand Down Expand Up @@ -151,7 +151,7 @@ def get_num_classes(
return num_classes


def _stable_1d_sort(x: torch, nb: int = 2049):
def _stable_1d_sort(x: Tensor, nb: Optional[int] = None) -> Tuple[Tensor, Tensor]:
"""
Stable sort of 1d tensors. Pytorch defaults to a stable sorting algorithm
if number of elements are larger than 2048. This function pads the tensors,
Expand All @@ -172,12 +172,13 @@ def _stable_1d_sort(x: torch, nb: int = 2049):
if x.ndim > 1:
raise ValueError('Stable sort only works on 1d tensors')
n = x.numel()
if n < nb:
x_max = x.max()
x = torch.cat([x, (x_max + 1) * torch.ones(nb - n, dtype=x.dtype, device=x.device)], 0)
if nb is not None:
Copy link

Choose a reason for hiding this comment

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

IMO we should just remove nb option. I don't see when you'd ever want it to be anything other than 2049. The padding is just a hack to get torch to sort the array in a stable manner (look at the docstring).

if n < nb:
x_max = x.max()
x = torch.cat([x, (x_max + 1) * torch.ones(nb - n, dtype=x.dtype, device=x.device)], 0)
n = min(nb, n)
x_sort = x.sort()
i = min(nb, n)
return x_sort.values[:i], x_sort.indices[:i]
return x_sort.values[:n], x_sort.indices[:n]


def apply_to_collection(
Expand Down