[Metrics] AUC reorder is unstable

[ddrevicky]

🐛 Bug

When auc(x, y, reorder=True) is called (True is the default) it reorders the xs and ys. It uses torch.argsort on x internally, which is unstable and does not take the second key y into account when sorting. In some cases this leads to reordering that changes the order of ys in an undesirable way resulting in incorrect metric computation.

fpr = torch.tensor([0.4167, 0.5000, 0.5000,
        0.5000, 0.5833, 0.6667, 0.6667, 0.6667, 0.7500, 0.7500, 0.7500, 0.8333,
        0.9167, 0.9167, 0.9167, 1.0000])
tpr = torch.tensor([0.1111, 0.1111, 0.2222,
        0.3333, 0.3333, 0.3333, 0.4444, 0.5556, 0.5556, 0.6667, 0.7778, 0.7778,
        0.7778, 0.8889, 1.0000, 1.0000])

auc_sk = torch.tensor(sk_auc(fpr, tpr)).float()
auc_reorder = auc(fpr, tpr, reorder=True)
auc_no_reorder = auc(fpr, tpr, reorder=False)

print("Sklearn", auc_sk.item())
print("PTL no reorder", auc_no_reorder.item(), "SK==PTL:", torch.allclose(auc_sk, auc_no_reorder))
print("PTL with reorder", auc_reorder.item(), "SK==PTL:", torch.allclose(auc_sk, auc_reorder))
print("unstable argsort: ", torch.argsort(fpr))

>>> Sklearn 0.324056476354599
>>> PTL no reorder 0.324056476354599 SK==PTL: True
>>> PTL with reorder 0.3240675926208496 SK==PTL: False
>>> unstable argsort:  tensor([ 0,  1,  2,  3,  4,  7,  6,  5,  8,  9, 10, 11, 12, 13, 14, 15])

See an example colab. I'm sure there is a more minimal example possible, this is what I encountered when implementing a different issue.

Expected behavior

Either a stable way of sorting across both x and y or removing the reorder argument entirely.

Environment

Colab.

[Borda]

@teddykoker mind have look? :]

[teddykoker]

Functional metrics... cc. @justusschock @SkafteNicki

[justusschock]

With PyTorch it is hard to have a more stable sorting. Currently. Not sure, if we should actually implement it manually or remove it.

@SkafteNicki do you think we actually need this?

[SkafteNicki]

IMO we should probably remove it. Moving forward with the metrics package we should only support features that are stable.

[ddrevicky]

You can assign this to me then. Should I just straight remove it or do you want to deprecate it?

[justusschock]

I think rather deprecate it first and then remove it in 1.1 or 1.2