metrics[0]: add per-image overlap (pimo) [GSoC 2023 @ OpenVINO] #1247

jpcbertoldo · 2023-08-09T08:12:39Z

Description

This is part of Anomaly Segmentation Metrics for Anomalib — GSoC 2023 @ OpenVINO.

Previously in #1220

Per-Image Overlap (PImO)

Adding the metric Per-Image Overlap (PImO) and its area under the curve (AUC).

At each given threshold:

X-axis: in-image FPR average on normal images (equivalent to the set FPR of normal images).
Y-axis: Overlap between the class 'anomalous' in the ground truth and the predicted masks (in-image TPR).

I choose the vocabulary 'overlap' insted of TPR because it echoes with the Per-Region Overlap (PRO).

binclf curve

The per-image overlap (tpr) needs the binary classification matrix curve (binclf curve), which has to be computed per image , while they must share the thresholds (Since the x-axis (FPR) is shared).

You'll find a weird-looking funciton using itertools to compute thos binclf curves, which i found to be much faster on CPU than the two alternatives in torchmetrics (the latter mentioned in the former's doc) -- image below shows some WIP comparisons; a proper version on the way.

However, there is a tensor.cpu().numpy() in there, and I only tested with tensors already in the CPU.
Perhaps when using tensors in the GPU that operation might make the fastest algorithm change?

Fixes # (issue)

Changes

Bug fix (non-breaking change which fixes an issue)
Refactor (non-breaking change which refactors the code base)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist

My code follows the pre-commit style and check guidelines of this project.
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing tests pass locally with my changes
I have added a summary of my changes to the CHANGELOG (not for minor changes, docs and tests).

)

* New printing stuff * Remove dead code + address codacy issues * Refactor try/except + log to comet/wandb during runs * pre-commit error * third-party configuration --------- Co-authored-by: Ashwin Vaidya <[email protected]>

* ignore mask check when dataset has only normal samples * update changelog

Revert "🚚 Refactor Benchmarking Script (openvinotoolkit#1216)" This reverts commit 784767f.

* Fix metadata path * Update benchmarking notebook

jpcbertoldo · 2023-08-15T11:18:51Z

there is an issue with the feature branch it seems, could you enable codacy?

jpcbertoldo · 2023-08-18T10:39:18Z

This coverage issue it because I saved the notebook with the kernel name in my loca environment (anomalib-dev-gsoc).
Can we ignore it for now and solve it in a later PR?

samet-akcay

@jpcbertoldo, thanks for this! I just have minor comments, majority of which are naming-related.

samet-akcay · 2023-08-22T09:50:42Z

src/anomalib/utils/metrics/__init__.py

@@ -20,9 +20,22 @@
 from .collection import AnomalibMetricCollection
 from .min_max import MinMax
 from .optimal_f1 import OptimalF1
+from .perimg import AUPImO, PerImageBinClfCurve, PImO


Can this be per_img instead?

Is it possible to name PerImageBinClfCurve something more explicit. BinClf may not be obvious initially. Do you think if PerImageBinaryClassificationCurve would be too verbose?

I used PerImageBinaryClassificationCurve at first but it feels very wordy and some lines will become tough to read.
binclf is what I found in torchmetrics so i copied them.

per_img i think it's ok; put it in the list of refactors.

samet-akcay · 2023-08-22T09:53:22Z

src/anomalib/utils/metrics/perimg/common.py

is common.py needed? Could these be in __init__.py. Or alternatively, can this be a utils.py?

Can you add doctoring and other headers as well?

is common.py needed? Could these be in __init__.py. Or alternatively, can this be a utils.py?

In __init__.py it would create circular imports (in future PRs).

common.py -> utils.py is fine IMO, but it will later have more things inside that are not really "utils". I'll add this to the refactors.

Can you add doctoring and other headers as well?

Sure. I'll add the docstrings, but what other headers specifically (apart from license)?

Can you add doctoring and other headers as well?

Sure. I'll add the docstrings, but what other headers specifically (apart from license)?

I reviewed and incremented module docstrings in a future PR.

jpcbertoldo@621f58e

src/anomalib/utils/metrics/perimg/pimo.py

samet-akcay · 2023-08-22T10:34:56Z

src/anomalib/utils/metrics/perimg/pimo.py

+
+# =========================================== METRICS ===========================================
+
+PImOResult = namedtuple(


do you have any specific motivation why use named tuple instead of data class. I was just curious :) Just preference or any other motivation?

Yes and more or less.

It's possible to just unpack the things, like

_, __, shared_fpr, tprs, image_classes = self.compute()

Also it's just a tuple, I think it makes user code less dependent on in-anomalib classes. I tend to use it for this type of case where I actually just want to return many things but there are so many that it's easier to document it as a namedtuple. I noticed scipy does something similar (?).

Otherwise I'm a big fan of dataclass : )

src/anomalib/utils/metrics/perimg/common.py

jpcbertoldo · 2023-08-22T13:36:55Z

List of refactors before merging feature branch:
https://github.com/jpcbertoldo/anomalib/blob/metrics/refactors/src/anomalib/utils/metrics/perimg/.refactors

Adds from this PR

1. rename `perimg` to `per_img`
  1.1 in functions
  1.2 the module
2. rename`common.py` to `utils.py`
3. add license header to files
4. remove `.ignore`
5. rename `atleast` to `at_least` (?)

jpcbertoldo · 2023-08-23T13:32:43Z

@samet-akcay i put the naming issues in the todo list for the final refactor
i think we can merge this and move to the next PR?

samet-akcay · 2023-08-23T13:53:56Z

@samet-akcay i put the naming issues in the todo list for the final refactor
i think we can merge this and move to the next PR?

Notebook tests are failing due to your notebook, but you could address that later too

jpcbertoldo and others added 30 commits July 27, 2023 23:34

add per-image overlap (pimo)

a67b35f

modif plot pimo curves

6376942

add warning about memory

d999729

tiny bug

3df9849

add tuto ipynb

2b16061

make image classes a return

cacb653

fix ipynb

56b0bcd

add tests for binclf curve

1476a56

add test to binclf

5557db1

add aupimo tests

70f9d37

ruff

0b05edd

Configure readthedocs via .readthedocs.yaml file (openvinotoolkit#1229

0c7444e

)

Update binclf_curve.py

b844261

🚚 Refactor Benchmarking Script (openvinotoolkit#1216)

784767f

* New printing stuff * Remove dead code + address codacy issues * Refactor try/except + log to comet/wandb during runs * pre-commit error * third-party configuration --------- Co-authored-by: Ashwin Vaidya <[email protected]>

Update CODEOWNERS

5a46d03

Enable training with only normal images for MVTec (openvinotoolkit#1241)

3ffc3f6

* ignore mask check when dataset has only normal samples * update changelog

Revert "🚚 Refactor Benchmarking Script" (openvinotoolkit#1239)

a62cd92

Revert "🚚 Refactor Benchmarking Script (openvinotoolkit#1216)" This reverts commit 784767f.

Update benchmarking notebook (openvinotoolkit#1242)

09ad1d4

* Fix metadata path * Update benchmarking notebook

add per-image overlap (pimo)

0b639d4

modif plot pimo curves

543db2b

add warning about memory

c8537c7

tiny bug

0cb153b

add tuto ipynb

c076165

make image classes a return

ba6a5a6

fix ipynb

974d3bb

add tests for binclf curve

1ff1832

add test to binclf

241fb2e

add aupimo tests

b8c588f

ruff

77355a7

Update binclf_curve.py

4b67ced

jpcbertoldo requested review from paularamo and yunchu as code owners August 9, 2023 08:16

samet-akcay added Metrics Metric Component. and removed CI Tests Notebooks Docs labels Aug 9, 2023

samet-akcay removed request for yunchu and paularamo August 9, 2023 08:27

Merge branch 'feature/pimo' into metrics/root

6516491

github-actions bot added Tests Notebooks and removed Metrics Metric Component. labels Aug 9, 2023

correct tests

55f84c8

jpcbertoldo added 6 commits August 15, 2023 13:29

add test

596ebad

fix test

0327861

add plots tests

b439e15

Merge branch 'feature/pimo' into metrics/root

35a91ef

correct codacy stuff

e2a5b91

correct codacy stuff

3d995e0

samet-akcay reviewed Aug 22, 2023

View reviewed changes

samet-akcay merged commit 103c22c into openvinotoolkit:feature/pimo Aug 23, 2023

jpcbertoldo changed the title ~~metrics[0]: add per-image overlap (pimo)~~ metrics[0]: add per-image overlap (pimo) [GSoC 2023 @ OpenVINO Aug 24, 2023

jpcbertoldo changed the title ~~metrics[0]: add per-image overlap (pimo) [GSoC 2023 @ OpenVINO~~ metrics[0]: add per-image overlap (pimo) [GSoC 2023 @ OpenVINO] Aug 24, 2023

jpcbertoldo deleted the metrics/root branch February 9, 2024 12:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

metrics[0]: add per-image overlap (pimo) [GSoC 2023 @ OpenVINO] #1247

metrics[0]: add per-image overlap (pimo) [GSoC 2023 @ OpenVINO] #1247

jpcbertoldo commented Aug 9, 2023 •

edited

Loading

jpcbertoldo commented Aug 15, 2023

jpcbertoldo commented Aug 18, 2023

samet-akcay left a comment

samet-akcay Aug 22, 2023

samet-akcay Aug 22, 2023

jpcbertoldo Aug 22, 2023

samet-akcay Aug 22, 2023

samet-akcay Aug 22, 2023

jpcbertoldo Aug 22, 2023

jpcbertoldo Aug 22, 2023 •

edited

Loading

jpcbertoldo Aug 22, 2023

samet-akcay Aug 22, 2023

jpcbertoldo Aug 22, 2023 •

edited

Loading

jpcbertoldo commented Aug 22, 2023

jpcbertoldo commented Aug 23, 2023

samet-akcay commented Aug 23, 2023


		# =========================================== METRICS ===========================================

		PImOResult = namedtuple(

metrics[0]: add per-image overlap (pimo) [GSoC 2023 @ OpenVINO] #1247

metrics[0]: add per-image overlap (pimo) [GSoC 2023 @ OpenVINO] #1247

Conversation

jpcbertoldo commented Aug 9, 2023 • edited Loading

Description

Per-Image Overlap (PImO)

binclf curve

Changes

Checklist

jpcbertoldo commented Aug 15, 2023

jpcbertoldo commented Aug 18, 2023

samet-akcay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpcbertoldo Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpcbertoldo Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

jpcbertoldo commented Aug 22, 2023

jpcbertoldo commented Aug 23, 2023

samet-akcay commented Aug 23, 2023

jpcbertoldo commented Aug 9, 2023 •

edited

Loading

jpcbertoldo Aug 22, 2023 •

edited

Loading

jpcbertoldo Aug 22, 2023 •

edited

Loading