Reformat iou [func] and add IoU class

[deng-cy]

What does this PR do?

This PR makes some changes related to iou:

• Change iou (functional) to be more flexible to input float value. The code is rewritten to use confusion matrix to get the score.
• Add a IoU class which functions the same as (new) iou.

I tested this class on my machine, but did not write a pytest file for IoU class.

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 your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
• Did you make sure to update the documentation with your changes?
• Did you write any new necessary tests?
• Did you verify new and existing tests pass locally with your changes?
• If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In in short, see following bullet-list:

• Is this pull request ready for review? (if not, please submit in draft mode)
• Check that all items from Before submitting are resolved
• Make sure the title is self explanatory and the description concisely explains the PR
• Add labels and milestones (and optionally projects) to the PR so it can be classified; Bugfixes should be including in bug-fix release milestones (m.f.X) and features should be included in (m.X.b) releases.

Did you have fun?

Make sure you had fun coding 🙃

[pep8speaks]

Hello @deng-cy! Thanks for updating this PR.

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

Comment last updated at 2021-01-08 12:59:07 UTC

[codecov]

Codecov Report

Merging #4704 (854e942) into release/1.2-dev (06668c0) will increase coverage by 0%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##           release/1.2-dev   #4704   +/-   ##
===============================================
  Coverage               93%     93%           
===============================================
  Files                  148     150    +2     
  Lines                10480   10490   +10     
===============================================
+ Hits                  9709    9719   +10     
  Misses                 771     771           

[SkafteNicki]

Smaller comments for now.
For testing, you should be able to test against sklearns jaccard score: https://scikit-learn.org/stable/modules/generated/sklearn.metrics.jaccard_score.html as it computes the same metric

[tadejsv]

A problem with this implementation is that it will not work for multi-label inputs (since it is based on the confusion matrix)

[deng-cy]

A problem with this implementation is that it will not work for multi-label inputs (since it is based on the confusion matrix)

I did not know that. This implementation passed all tests. Can you give an example?

[tadejsv]

It passed all tests because currently the tests are not really well done (not your fault, just the state of the testing library). What is done in these tests is that the vectors are unrolled (with view(-1)) into 1d vectors, losing all structure.

As for an example: try this [edited to include average='macro']

import numpy as np
from sklearn.metrics import jaccard_score
y_true = np.array([[0, 1, 1],
                   [1, 1, 0]])
y_pred = np.array([[1, 1, 1],
                   [1, 0, 0]])

jaccard_score(y_true, y_pred, average='macro')

This should give you 0.6666 (it's the example from sklearn documentation)
Now try what you get with your implementation (try different reduction methods), i.e.

import torch
from pytorch_lightning.metrics.functional import iou
y_true = torch.tensor([[0, 1, 1],
                       [1, 1, 0]])
y_pred = torch.tensor([[1, 1, 1],
                       [1, 0, 0]])

iou(y_pred, y_true)

[tadejsv]

This problem isn't something you can easily fix, because the confusion matrix can not deal with multi-label inputs. You have to use a different approach - computing the number of true positives, false positives, etc. directly. I'm working on a PR for this :)

[deng-cy]

It passed all tests because currently the tests are not really well done (not your fault, just the state of the testing library). What is done in these tests is that the vectors are unrolled (with view(-1)) into 1d vectors, losing all structure.

As for an example: try this [edited to include average='macro']

import numpy as np
from sklearn.metrics import jaccard_score
y_true = np.array([[0, 1, 1],
                   [1, 1, 0]])
y_pred = np.array([[1, 1, 1],
                   [1, 0, 0]])

jaccard_score(y_true[0], y_pred[0], average='macro')

This should give you 0.6666 (it's the example from sklearn documentation)
Now try what you get with your implementation (try different reduction methods), i.e.

import torch
from pytrorch_lightning.metrics.functional import iou
y_true = torch.tensor([[0, 1, 1],
                       [1, 1, 0]])
y_pred = torch.tensor([[1, 1, 1],
                       [1, 0, 0]])

iou(y_pred, y_true)

The previous (current) implementation uses what you described, but I think our outputs are the same. The sk one treats each column as a class, and calculates accuracy of each class. I feel it can be substituted by accuracy?

[tadejsv]

Your PR still uses the same "unrolling": please see the _multilabel_sk_metric function in test_iou.py. This why tests are passing even though the code doesn't produce the right results.

The outputs are not the same. I tried the example I gave you for your PR, and the results are (for reduction method):

• elementwise_mean: 0.4667
• sum: 0.9333
• none: [0.3333, 0.6000]

None of them gives the correct result, which is 0.6666

Sklearn's jaccard_score does not calculate the accuracy of each class - it calculates the IoU of each class. There isn't even a definition for what an "accuracy" of a class should be :)

The sklearn example is simple enough to do by hand, if you don't believe me.

[deng-cy]

I meant my output was the same as the current implementation (which used your way and I deleted), not the sklearn function. The reason I felt it looked like accuracy is that

>>> sk_jaccard_score(pred, target, average=None)
[0.5 0.5 1. ]

seems accuracy to me lol.

I understand it may be different from accuracy (I have not dug into it), but I think it is a choice of what function we want here. At least from the current implementation, I do not think such multilabel is considered. You are one of the code owners, of course you should be the one to decide what to implement here :D

Also, if you want to implement multilabel, is it possible to achieve this function by converting one-hot to categorical and proceed with confusion matrix?

[tadejsv]

I checked and you are right that the current implementation also gets it wrong.

You can check out sklearn's great used guide for a definition of IoU and accuracy. IoU is defined as tp / (tp + fp + fn). You do get equivalence between this and (tp + tn)/(tp + fp + tn + fn), which you could call "class accuracy", in a simple 2 sample case, but generally it is not true.

I am not one of the code owners :) But in my opinon IoU should definetly support multi-label inputs

And I don't think you can use any tricks to get multi-label data into any kind of meaningful confusion matrix representation.

[Borda]

still need to handle back-compatibility as shell linking the new function location

[Borda]

I have not check the computation, but still, there are some minor thing for propper deprecation

[Borda]

nice job!