Changes for Test Differentiability [2/n]

[bhadreshpsavani]

Before submitting

• Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure to update the docs?
• Did you write any new necessary tests?

What does this PR do?

Fixes #26

PR review

@SkafteNicki

Note:

• After My changes in the Test File, Test Cases that I wrote were failing for a few scenarios
• I value of Differentiability Property might not be very accurate. I wrote it according to my understanding and few internet searches

[pep8speaks]

Hello @bhadreshpsavani! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-04-30 13:12:29 UTC

[codecov]

Codecov Report

Merging #178 (bb57cb5) into master (8d9c592) will increase coverage by 0.01%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master     #178      +/-   ##
==========================================
+ Coverage   96.65%   96.67%   +0.01%     
==========================================
  Files         180      180              
  Lines        5744     5770      +26     
==========================================
+ Hits         5552     5578      +26     
  Misses        192      192              

Flag	Coverage Δ
Linux	79.65% <63.15%> (-0.08%)	⬇️
Windows	79.65% <63.15%> (-0.08%)	⬇️
cpu	96.67% <95.65%> (+0.01%)	⬆️
gpu	96.67% <ø> (+0.01%)	⬆️
macOS	96.67% <95.65%> (+0.01%)	⬆️
pytest	96.67% <95.65%> (+0.01%)	⬆️
python3.6	95.57% <94.73%> (+0.01%)	⬆️
python3.8	96.63% <95.65%> (+0.01%)	⬆️
python3.9	96.53% <95.65%> (+0.01%)	⬆️
torch1.3.1	95.57% <94.73%> (+0.01%)	⬆️
torch1.4.0	95.70% <95.65%> (+0.01%)	⬆️
torch1.8.1	96.53% <95.65%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
torchmetrics/functional/classification/auc.py	87.50% <90.90%> (+0.54%)	⬆️
torchmetrics/classification/auc.py	100.00% <100.00%> (ø)
torchmetrics/classification/auroc.py	92.50% <100.00%> (+0.60%)	⬆️
torchmetrics/classification/average_precision.py	100.00% <100.00%> (ø)
torchmetrics/classification/cohen_kappa.py	100.00% <100.00%> (ø)
__w/2/s/torchmetrics/classification/auc.py	100.00% <0.00%> (ø)
__w/2/s/torchmetrics/classification/cohen_kappa.py	100.00% <0.00%> (ø)
...s/torchmetrics/classification/average_precision.py	100.00% <0.00%> (ø)
.../2/s/torchmetrics/functional/classification/auc.py	87.50% <0.00%> (+0.54%)	⬆️
__w/2/s/torchmetrics/classification/auroc.py	92.50% <0.00%> (+0.60%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d9c592...bb57cb5. Read the comment docs.

[SkafteNicki]

some tests are failing because this assert cannot handle metrics that returns multiple values.
https://github.com/PyTorchLightning/metrics/blob/c1ae5ace2dc03525720ab3a867ce35fe50a2d276/tests/helpers/testers.py#L465
should be changed to something like:

if isinstance(out, list):
    assert all(metric.is_differentiable == o.requires_grad for o in out)
else:
    assert metric.is_differentiable == out.requires_grad

[SkafteNicki]

After looking a bit on the failing tests it seems that:

• AUC should in general be differentiable and the False should be changed to True
• However, for half of the test cases (those with batch size 4049 https://github.com/PyTorchLightning/metrics/blob/cceced613f4323a1f5124099a969f2cf32a80d7e/tests/classification/test_auc.py#L44) the grad check fails, indicating that the gradient calculation in pytorch is wrong. It makes sense that it only fails for large inputs as we know pytorch is using different implementations in torch.sort depending on the size of the input:
  https://github.com/pytorch/pytorch/blob/a24163a95edb193ff7b06e98cd69bf7cfd4c0d2f/aten/src/THC/generic/THCTensorSort.cu#L306-L334

@bhadreshpsavani could you add a pytest.skipif to the differentiability test of AUC such that it is skipped if the input is larger than 2048?

[Borda]

@bhadreshpsavani mind check the failing tests?

[maximsch2]

For the pytorch bug - it would be great to have a small isolated repro to file as an issue for pytorch.

On AUC - is it really meaningful to treat it as differentiable? What tasks will do that?

[bhadreshpsavani]

Hi @Borda,
I will make changes according to the suggestion of @SkafteNicki,
Let me know if anything I need to change.

[bhadreshpsavani]

Hi @SkafteNicki,
I made changes as per your suggestions
Eventhough i add this line @pytest.mark.skipif(x.shape[1] > 2048) in metrics/tests/classification/test_auc.py it still fails.

[SkafteNicki]

After searching the internet, AUC indeed is described as an non-differentiable function and it must therefore be our specific implementation in pytorch that allows gradient to be backprop.
I have inserted a torch.no_grad to secure that the function is non-differentiable which also fixes the tests.

[Borda]

LGTM