Open djdameln opened 1 week ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
Attention: Patch coverage is 94.67456%
with 9 lines
in your changes missing coverage. Please review.
Please upload report for BASE (
feature/design-simplifications@8543e24
). Learn more about missing BASE report.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@abc-125 @alexriedel1 @blaz-r @jpcbertoldo, if/when you have some time, would you like to review this PR? It introduces some major changes. It would be great to get your perspective.
I'll check this tomorrow in the morning.
I think this looks great ๐. I added some small comments.
I do have two main things to point out:
- Similar as the one from Ashwin - is there no way to pass the metrics just by name now (CLI, config)?
- About the default evaluator inside anomaly_module. This change might not be completely compatible with current configs due to early stopping and behavior of existing validation (more details about this in the code comment).
@blaz-r, thanks for your review. This PR is part of a greater initiative, which might potentially break things again. That's why we started to think of this as anomalib v2. Here is a proposal showing how Anomalib base model would look like
Cool, thanks for the shared info.
Just on a side note, if you add AUPIMO
to this, i would suggest a small change in the design.
Remove the option return_average
that we added in the last minute, and instead create another classe AverageAUPIMO(AUPIMO)
.
It would have something like
def compute(...):
_, aupimo_result = super().compute(...)
# normal images have NaN AUPIMO scores
is_nan = torch.isnan(aupimo_result.aupimos)
return aupimo_result.aupimos[~is_nan].mean()
which is currently done in an if
inside AUPIMO
.
๐ Description
Evaluator
class which replacesMetricsCallback
.PostProcessor
, the evaluator inherits fromCallback
to allow accessing the val and test outputs, and fromnn.Module
to store the metrics as a submodule in the Lightning model.AnomalibMetric
class, which addsfields
arg and attribute to existing metrics from torchmetrics. By inheriting fromAnomalibMetric
and a givenMetric
subclass, a new Anomalib-compatible version of the metric is created. When instantiating the metric, the user must specify the batch fields that will be used to update the metric. During update, we only need to pass the batch to the update method.Some examples:
Other changes:
**kwargs
from lightning models toAnomalyModule
base classโจ Changes
Select what type of change your PR is:
โ Checklist
Before you submit your pull request, please make sure you have completed the following steps:
For more information about code review checklists, see the Code Review Checklist.