Pytorch-lightning: [Metrics] AUC reorder is unstable

Created on 19 Oct 2020  路  6Comments  路  Source: PyTorchLightning/pytorch-lightning

馃悰 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.

Metrics bug / fix help wanted

All 6 comments

@teddykoker mind have look? :]

Functional metrics... cc. @justusschock @SkafteNicki

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?

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

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

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

Was this page helpful?
0 / 5 - 0 ratings

Related issues

anthonytec2 picture anthonytec2  路  3Comments

edenlightning picture edenlightning  路  3Comments

monney picture monney  路  3Comments

DavidRuhe picture DavidRuhe  路  3Comments

williamFalcon picture williamFalcon  路  3Comments