-
Notifications
You must be signed in to change notification settings - Fork 705
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
Feature/dick/anomaly score normalization #35
Feature/dick/anomaly score normalization #35
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @djdameln! Looks really good, only minor stuff.
anomalib/core/callbacks/__init__.py
Outdated
@@ -9,6 +9,7 @@ | |||
|
|||
from .compress import CompressModelCallback | |||
from .model_loader import LoadModelCallback | |||
from .normalization import NormalizationCallback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think NormalizationCallback
is not sufficient to understand what the callback does. I think it should be something like ScoreNormalizationCallback
or something similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I tried AnomalyScoreNormalizationCallback
but found it too verbose. ScoreNormalizationCallback
would be better, though maybe a bit vague. How about OutputNormalizationCallback
, because it normalizes the outputs of the model?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to OutputNormalizationCallback
. Let me know what you think
tests/core/callbacks/normalization_callback/test_normalization_callback.py
Outdated
Show resolved
Hide resolved
Thanks for the review @samet-akcay. I addressed your comments and updated the PR |
requirements/requirements.txt
Outdated
@@ -1,8 +1,9 @@ | |||
pillow==8.3.2 | |||
nncf>=2.0.0 | |||
pytorch-lightning==1.3.6 | |||
torch==1.8.1 | |||
torchvision==0.9.1 | |||
-f https://download.pytorch.org/whl/torch_stable.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to remove this line too.
from torch.distributions import LogNormal, Normal | ||
|
||
|
||
class OutputNormalizationCallback(Callback): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about the name of your PR? :) Like AnomalyScoreNormalizationCallback
or ScoreNormalizationCallback
. It doesn't matter if it's verbose. What's more important is its readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done with the second round. This will be super cool once merged.
This reverts commit 5d46732.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the effort @djdameln! This is a crucial addition!
Maybe we could use version 0.2.3 for this one? |
…o feature/dick/anomaly-score-normalization
|
||
|
||
def test_normalizer(): | ||
config = get_configurable_parameters(model_config_path="anomalib/models/padim/config.yaml") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add get_dataset_path()
to config.dataset.path
? Without this, the CI is downloading the entire dataset again in the current directory. Due to the permissions of the original files, the CI cannot remove the dataset once the test runs.
…:openvinotoolkit/anomalib into feature/dick/anomaly-score-normalization
Description
Changes
NormalizationCallback
, which normalizes the predicted scores and error maps based on the distribution of normal data in the training set.TorchMetrics
classes, which de-clutters the base anomaly module and allows automatic saving and loading of the values in the model's state dict.F1
class configured with the computed threshold, instead ofOptimalF1
.FeatureExtractor
was replaced by torchvision'screate_feature_extractor
. For this I had to bump up the torch and torchvision versions.Checklist: