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.

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

Use pytorch-like ignore_index instead of remove_bg for IoU #2736

Closed
remisphere opened this issue Jul 28, 2020 · 5 comments · Fixed by #3098
Closed

Use pytorch-like ignore_index instead of remove_bg for IoU #2736

remisphere opened this issue Jul 28, 2020 · 5 comments · Fixed by #3098
Labels
feature Is an improvement or enhancement help wanted Open to be worked on

Comments

@remisphere
Copy link

remisphere commented Jul 28, 2020

🚀 Feature

PL's implementation of the IoU metric has a remove_bg flag that allows to ignore the background class.
I propose changing it to be the same as PyTorch's loss functions,
that is an ignore_index argument that takes the index of the class to be ignored.

Motivation for the proposal

Currently when remove_bg is set to True, the background is expected to be associated to index 0.

  • This is not a constant among datasets, for example Cityscapes uses index 255 for its ignored semantic segmentation class.
  • Pytorch's loss functions for classification that allow to ignore a class do it with an ignore_index argument, like CrossEntropyLoss.

That means that when using those the way they are intended to be, you have to manually move the ignored class' channel at the beginning of the tensor in-between the loss and metric computation, and I don't like having that in the middle of my training step.

Pitch

I'd like to have the same interface as PyTorch's loss functions with the ignore_index argument.

On a side note, I also think that the documentation should display the expected shape of inputs, like PyTorch' loss functions, and not keep quiet about the automatic adaptation to cases when the prediction is given in one-hot encoding or as an index array.

Alternatives

Appart from modifying the data between loss an metric computation in the training step, it should be possible to do it at the data-set creation level, with custom transformations, and then use an ignore_index=0 for the loss function.

For the first method, this is what I add:

# set ignored class at pos 0
y_pred = torch.roll(y_pred, shifts=1, dims=1)
y = torch.add(y, 1)
y[y == 256] = 0
@remisphere remisphere added feature Is an improvement or enhancement help wanted Open to be worked on labels Jul 28, 2020
@github-actions
Copy link
Contributor

Hi! thanks for your contribution!, great first issue!

@edenlightning
Copy link
Contributor

@j-dsouza mind taking a look?

@remisphere
Copy link
Author

remisphere commented Jul 30, 2020

For information, I discussed a bit about IoU's usage in #2753 ; I'm not sure, but I think the present issue is still relevant.

@Borda Borda added the Metrics label Jul 31, 2020
@Borda Borda changed the title [Metrics] Use pytorch-like ignore_index instead of remove_bg for IoU Use pytorch-like ignore_index instead of remove_bg for IoU Jul 31, 2020
@Borda
Copy link
Member

Borda commented Jul 31, 2020

cc: @justusschock @SkafteNicki

@SkafteNicki
Copy link
Member

I would gladly review a PR that implements this. The idea with the metric package is that it should be as close as possible to native torch.

abrahambotros added a commit to abrahambotros/pytorch-lightning that referenced this issue Sep 3, 2020
Fixes Lightning-AI#2736

- Rename IoU metric argument from `remove_bg` -> `ignore_index`.
- Accept an optional int class index to ignore, instead of a bool and
  instead of always assuming the background class has index 0.
- If given, ignore the class index when computing the IoU output,
  regardless of reduction method.
abrahambotros added a commit to abrahambotros/pytorch-lightning that referenced this issue Sep 4, 2020
Fixes Lightning-AI#2736

- Rename IoU metric argument from `remove_bg` -> `ignore_index`.
- Accept an optional int class index to ignore, instead of a bool and
  instead of always assuming the background class has index 0.
- If given, ignore the class index when computing the IoU output,
  regardless of reduction method.
abrahambotros added a commit to abrahambotros/pytorch-lightning that referenced this issue Sep 4, 2020
Fixes Lightning-AI#2736

- Rename IoU metric argument from `remove_bg` -> `ignore_index`.
- Accept an optional int class index to ignore, instead of a bool and
  instead of always assuming the background class has index 0.
- If given, ignore the class index when computing the IoU output,
  regardless of reduction method.
Borda pushed a commit to abrahambotros/pytorch-lightning that referenced this issue Sep 4, 2020
Fixes Lightning-AI#2736

- Rename IoU metric argument from `remove_bg` -> `ignore_index`.
- Accept an optional int class index to ignore, instead of a bool and
  instead of always assuming the background class has index 0.
- If given, ignore the class index when computing the IoU output,
  regardless of reduction method.
abrahambotros added a commit to abrahambotros/pytorch-lightning that referenced this issue Sep 8, 2020
Fixes Lightning-AI#2736

- Rename IoU metric argument from `remove_bg` -> `ignore_index`.
- Accept an optional int class index to ignore, instead of a bool and
  instead of always assuming the background class has index 0.
- If given, ignore the class index when computing the IoU output,
  regardless of reduction method.
abrahambotros added a commit to abrahambotros/pytorch-lightning that referenced this issue Sep 14, 2020
Fixes Lightning-AI#2736

- Rename IoU metric argument from `remove_bg` -> `ignore_index`.
- Accept an optional int class index to ignore, instead of a bool and
  instead of always assuming the background class has index 0.
- If given, ignore the class index when computing the IoU output,
  regardless of reduction method.
abrahambotros added a commit to abrahambotros/pytorch-lightning that referenced this issue Sep 15, 2020
Fixes Lightning-AI#2736

- Rename IoU metric argument from `remove_bg` -> `ignore_index`.
- Accept an optional int class index to ignore, instead of a bool and
  instead of always assuming the background class has index 0.
- If given, ignore the class index when computing the IoU output,
  regardless of reduction method.
Borda pushed a commit that referenced this issue Sep 17, 2020
* Fix IoU score for classes not present in target or pred

Fixes #3097

- Allow configurable not_present_score for IoU for classes
  not present in target or pred. Defaults to 1.0.
- Also allow passing `num_classes` parameter through from iou
  metric class down to its underlying functional iou
  call.

* Changelog: move IoU not-present score fix to [unreleased]

* IoU: avoid recomputing class presence in target and pred

Use already-computed support, true positives, and false positives to
determine if a class is not present in either target or pred.

* Test IoU against sklearn jaccard_score

Also add TODO to test our IoU's not_present_score against sklearn's
jaccard_score's zero_division when it beecomes available.

* IoU: remove_bg -> ignore_index

Fixes #2736

- Rename IoU metric argument from `remove_bg` -> `ignore_index`.
- Accept an optional int class index to ignore, instead of a bool and
  instead of always assuming the background class has index 0.
- If given, ignore the class index when computing the IoU output,
  regardless of reduction method.

* Improve documentation for IoU not_present_score

* Update default IoU not_present_score to 0.0

* Add note about IoU division by zero

* Rename IoU not_present_score -> absent_score

* Update IoU absent score changelog wording

* Condense IoU absent_score argument docstring

* Remove unnecessary IoU ignore_index comment

* docstrings

* isort

* flake8

* Fix test of IoU against sklearn jaccard

Use macro instead of micro averaging in sklearn's jaccard score, to
match multi-class IoU, which conventionally takes per-class scores
before averaging.

Co-authored-by: rohitgr7 <rohitgr1998@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement help wanted Open to be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants