Add metric visualizations #429

ORippler · 2022-07-11T14:16:07Z

Description

This PR:

Adds 2 additional metrics:
- AUPR
- AUPRO
Adds plots for all metrics by refactoring VisualizerCallback into VisualizerCallbackBase and VisualizerCallbackimage, and by introducing a new VisualizerCallbackMetric class

Fixes Add plots and additional metrics #420 (issue)

Changes

Bug fix (non-breaking change which fixes an issue)
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

* `VisualizerCallback` is refactored into a base `VisualizerCallbackBase` class which is used to construct `VisualizerCallbackImage` via inheritance. * Fix bug in `VisualizerCallback.on_test_end`, where `pl_module.logger` was accessed instead of iterating over `trainer.loggers` (wandb errort silently before iirc) * Skeleton for `VisualizerCallbackMetric` is added

ORippler · 2022-07-11T14:17:08Z

anomalib/utils/callbacks/visualizer_base.py

+        for logger in trainer.loggers:
+            if isinstance(logger, AnomalibWandbLogger):
+                logger.save()


Unsure whether it's "bad practice" to call .save on a AnomalibWandbLogger multiple times, as is done currently. Any insights?

It has been a while since I have looked into AnomalibWandbLogger but ideally it should only be called once at the very end. This is explained in the comment above This is because logging as a single batch ensures that all images appear as part of the same step.. In case it is being called multiple times then my guess is that the isinstance falls back to the base class and probably logger ends up always being an instance of AnomalibWandbLogger (LightningLogger more specifically). Maybe a good idea would be to use type() but mypy or pylint might complain.

I opted with clearing AnomalibWandbLogger.image_list on save. Thus, all predictions are in the first step, and all metrics in the second step (though still under the same namespace)

samet-akcay · 2022-07-11T16:45:29Z

@ORippler, thanks a lot for your contribution! Just so know, code quality checks have failed due to number of reasons

anomalib/utils/cli/cli.py:21:0: E0611: No name 'VisualizerCallback' in module 'anomalib.utils.callbacks' (no-name-in-module)
notebooks/200_models/201_fastflow.ipynb:cell_1:17:0: E0611: No name 'VisualizerCallback' in module 'anomalib.utils.callbacks' (no-name-in-module)

For more details, you could refer to this.

Will you be able to address this ?

ORippler · 2022-07-12T06:17:49Z

@ORippler, thanks a lot for your contribution! Just so know, code quality checks have failed due to number of reasons
1. anomalib/utils/cli/cli.py:21:0: E0611: No name 'VisualizerCallback' in module 'anomalib.utils.callbacks' (no-name-in-module)

2. notebooks/200_models/201_fastflow.ipynb:cell_1:17:0: E0611: No name 'VisualizerCallback' in module 'anomalib.utils.callbacks' (no-name-in-module)
For more details, you could refer to this.

Will you be able to address this ?

Sure, once I get all the functionality implemented.

PyTorch provides a function for logging Figure objects, so lets not reinvent the wheel. https://pytorch.org/docs/stable/tensorboard.html?highlight=add_figure#torch.utils.tensorboard.writer.SummaryWriter.add_figure

Otherwise, matplotlib starts to complain and objects aren't gced properly

`Visualizer` is needed by all Callbacks for writing images to disk

Every metric that should be visualized needs to implement its own `generate_figure` function, and the resulting plot is then saved by `VisualizerCallbackMetric`

…alib-1 into feature/metrics_visualizer

* Needed to change signature of `AnomalyModule._collect_outputs` for pixel-wise metrics as we need spatial dimensions for connected-componenta-analysis in AUPRO * Add AUPRO, which uses kornia and the fact that per-region overlap == per-region tpr for fast AUPRO computation * Updated docstrings for AUPR/AUROC

Due to bugs in CLI, feature could not be tested well

ORippler · 2022-07-12T15:25:43Z

@samet-akcay

I got the core functionality implemented and tested it locally so far. Some points from my side:

While I confirmed that metric-images are saved to tensorboard via the following snippet:

import cv2
import numpy as np
from matplotlib import pyplot as plt
from tensorboard.backend.event_processing.event_accumulator import EventAccumulator

acc = EventAccumulator("PATH")
acc.Reload()

for tag in acc.Tags()["images"]:
    if "image_" in tag or "pixel_" in tag:
        events = acc.Images(tag)

        for index, event in enumerate(events):
            s = np.frombuffer(event.encoded_image_string, dtype=np.uint8)
            image = cv2.imdecode(s, cv2.IMREAD_COLOR)
            image = cv2.cvtColor(image, cv2.COLOR_BGR2RGB)
            plt.imshow(image)
            plt.show()

they don't necessarily all appear when I open a tensorboard on the folder.

I did not test the changes to CLI, as I did not manage to call anomalib test after training (project.path is no longer in the namespace and cannot be provided via anomalib test --config --project.path. Therefore, a new time-stamped folder ist generated instead...)
instead of testing for presence of generate_figure, one could potentially generate an AnomalibMetric class with an abstractmethod, and let our own metrics inherit from that class (and subsequently test via isinstance(AnomalibMetric)).
The current implementation results in two calls to Metric._compute as opposed to the singular call prior. While this is not a bottleneck (at least not on an RTX 3090), one could nonetheless think of storing results from the first call and simply returning them instead.

review-notebook-app · 2022-07-13T06:51:04Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

ashwinvaidya17

Thanks for the efforts!

ashwinvaidya17 · 2022-07-13T11:32:15Z

anomalib/utils/callbacks/visualizer_metric.py

+                metrics = getattr(pl_module, metric_type)
+                for metric in metrics.values():
+                    # `generate_figure` needs to be defined for every metric that should be plotted automatically
+                    if hasattr(metric, "generate_figure"):


I agree maybe having a base metric with generate_figure as an abstract method would be a cleaner approach. But we can address this in a different pr

anomalib/utils/callbacks/visualizer_base.py

Since unique fpr/tpr curves are generated for each label, we cannot use fpr_index across calls ro `roc`. Instead, we now bilinearly resample generated pro curves at `fpr <= self.fpr_limit` to fixed sampling points, and then aggregate over the resampled curve.

We follow a scheme similar to the ROC curve, where the PRO curve is composed of per-region TPR plotted against global FPR.

ORippler · 2022-07-13T12:54:52Z

In addition to bugfixing the PRO-calculation, i passed the mypy checks and clarified wording.

Btw, mypy behaves differently when called with tox vs via the pre-commit hook (which didn't threw the errors I now fixed), at least in the provided Dockerfile

samet-akcay · 2022-07-13T13:29:45Z

In addition to bugfixing the PRO-calculation, i passed the mypy checks and clarified wording.

Btw, mypy behaves differently when called with tox vs via the pre-commit hook (which didn't threw the errors I now fixed), at least in the provided Dockerfile

Thanks for the update. We'll have a look at this

samet-akcay

@ORippler, thanks again for such contribution! I only have a single comment regarding the naming. Other than, that looks great!

anomalib/utils/callbacks/__init__.py

We now prepend the last part, i.e. `VisualizerCallbackBase` -> `BaseVisualizerCallback`

ORippler · 2022-07-13T16:06:53Z

Alright so the test for ganomaly should pass now as well (There were a couple of bugs in visualizer for the classification task as well as in the config for ganomaly, which were unrelated to my PR).

samet-akcay · 2022-07-13T16:12:05Z

Alright so the test for ganomaly should pass now as well (There were a couple of bugs in visualizer for the classification task as well as in the config for ganomaly, which were unrelated to my PR).

Thanks, yeah, I also noticed the classification bug in the visualizer yesterday. Will fix it in a separate PR

samet-akcay

Many thanks for your effort and valuable contribution!

anomalib/post_processing/visualizer.py

ORippler commented Jul 11, 2022

View reviewed changes

Merge branch 'development' into feature/metrics_visualizer

cfd92b1

ORippler added 9 commits July 12, 2022 06:36

Log Figure with built-in SummaryWriter funtcion

302a728

PyTorch provides a function for logging Figure objects, so lets not reinvent the wheel. https://pytorch.org/docs/stable/tensorboard.html?highlight=add_figure#torch.utils.tensorboard.writer.SummaryWriter.add_figure

Clear AnomalibWandbLogger.image_list on save

fe19315

Explicitly close figure in ImageGrid.generate

b4bbde7

Otherwise, matplotlib starts to complain and objects aren't gced properly

Shift Visualizer to VisualizerCallbackBase

c0d7f59

`Visualizer` is needed by all Callbacks for writing images to disk

Add first metric visualization

6fe5999

Every metric that should be visualized needs to implement its own `generate_figure` function, and the resulting plot is then saved by `VisualizerCallbackMetric`

Merge branch 'feature/metrics_visualizer' of github.com:ORippler/anom…

cee564f

…alib-1 into feature/metrics_visualizer

Merge branch 'development' into feature/metrics_visualizer

7b20171

Add AUPR metric

01286f4

Make AUPR accessible

f9522a0

samet-akcay mentioned this pull request Jul 12, 2022

Where can I save the segmentation results separately #294

Closed

ORippler added 2 commits July 12, 2022 13:59

Adjust tests, CLI and notebooks

bbbb188

Due to bugs in CLI, feature could not be tested well

ORippler marked this pull request as ready for review July 12, 2022 15:13

ORippler requested a review from ashwinvaidya17 July 12, 2022 15:26

Directly access AnomalyModule's metrics

2b6a391

ashwinvaidya17 approved these changes Jul 13, 2022

View reviewed changes

ORippler added 5 commits July 13, 2022 11:40

Add types as required by mypy

0da986f

Improve variable naming/wording in AUPRO

30b1c17

We follow a scheme similar to the ROC curve, where the PRO curve is composed of per-region TPR plotted against global FPR.

Move visualizer callbacks to their own module

4c5a795

Merge branch 'development' into feature/metrics_visualizer

87b3a2a

ORippler requested a review from ashwinvaidya17 July 13, 2022 12:55

samet-akcay reviewed Jul 13, 2022

View reviewed changes

anomalib/utils/callbacks/__init__.py Outdated Show resolved Hide resolved

ORippler added 2 commits July 13, 2022 14:46

Rename VisualizerCallbacks

469f4ef

We now prepend the last part, i.e. `VisualizerCallbackBase` -> `BaseVisualizerCallback`

Fix errors in visualizer and in ganomaly config

e2cf6c2

ORippler added 2 commits July 13, 2022 16:07

Revert change in max_epoch default for ganomaly

1dacd8a

Also revert change in default model

43d8e4a

samet-akcay approved these changes Jul 13, 2022

View reviewed changes

samet-akcay merged commit ba27019 into openvinotoolkit:development Jul 13, 2022

simon376 reviewed Jul 18, 2022

View reviewed changes

anomalib/post_processing/visualizer.py Show resolved Hide resolved

ORippler mentioned this pull request Jul 18, 2022

Add unit-tests for AUPRO metric #443

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metric visualizations #429

Add metric visualizations #429

ORippler commented Jul 11, 2022 •

edited

Loading

ORippler Jul 11, 2022

ashwinvaidya17 Jul 11, 2022 •

edited

Loading

ORippler Jul 12, 2022 •

edited

Loading

samet-akcay commented Jul 11, 2022

ORippler commented Jul 12, 2022

ORippler commented Jul 12, 2022 •

edited

Loading

review-notebook-app bot commented Jul 13, 2022

ashwinvaidya17 left a comment

ashwinvaidya17 Jul 13, 2022

ORippler commented Jul 13, 2022

samet-akcay commented Jul 13, 2022

samet-akcay left a comment

ORippler commented Jul 13, 2022

samet-akcay commented Jul 13, 2022

samet-akcay left a comment

Add metric visualizations #429

Add metric visualizations #429

Conversation

ORippler commented Jul 11, 2022 • edited Loading

Description

Changes

Checklist

ORippler Jul 11, 2022

Choose a reason for hiding this comment

ashwinvaidya17 Jul 11, 2022 • edited Loading

Choose a reason for hiding this comment

ORippler Jul 12, 2022 • edited Loading

Choose a reason for hiding this comment

samet-akcay commented Jul 11, 2022

ORippler commented Jul 12, 2022

ORippler commented Jul 12, 2022 • edited Loading

review-notebook-app bot commented Jul 13, 2022

ashwinvaidya17 left a comment

Choose a reason for hiding this comment

ashwinvaidya17 Jul 13, 2022

Choose a reason for hiding this comment

ORippler commented Jul 13, 2022

samet-akcay commented Jul 13, 2022

samet-akcay left a comment

Choose a reason for hiding this comment

ORippler commented Jul 13, 2022

samet-akcay commented Jul 13, 2022

samet-akcay left a comment

Choose a reason for hiding this comment

ORippler commented Jul 11, 2022 •

edited

Loading

ashwinvaidya17 Jul 11, 2022 •

edited

Loading

ORippler Jul 12, 2022 •

edited

Loading

ORippler commented Jul 12, 2022 •

edited

Loading