-
Notifications
You must be signed in to change notification settings - Fork 709
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
Update EfficientAD #1143
Update EfficientAD #1143
Conversation
Hello @nelson1425 and thanks for this.
I've seen that you also have your own implementation, which works a bit better, especially the M sized model. Do you have any more ideas where this discrepancy comes from in Anomalib? Thanks again for all this, and I hope that the above can be resolved as well as other changes verified by other contributors. |
@blaz-r Hello and thank you for your comments. Regarding the validation set: yes, this is an interesting question. Most anomalib methods have |
The reason why the other github implementations of EfficientAD cannot be used as they are in anomlib is, that anomalib uses albumentations for all the image transformation like resizing, cropping, gray. The Github implementations however use torchvision. Both have slightly different algorithms in their image transformation function that reduce the accuracy if you train a teacher model on torchvision transforms but then use albumentations transforms. Also, the pre-trained ResNet101 that is used to teach the teacher is trained on torchvision transforms which causes additional differences. I have trained the teacher model using albumentations but still cannot reach the paper accuracy in some classes. |
# we do this by sampling random elements of maps_flat because then | ||
# the locations of the quantiles (90% and 99.5%) will still be | ||
# valid even though they might not be the exact quantiles. | ||
max_input_size = 16777216 |
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 did you get that number? Is it an arbitrary choice?
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.
It is 16 * 1024 * 1024 and is the actual limit of what torch.quantile can handle.
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.
Ok, maybe you can use 2**24 here and reference the line in pytorch that causes the limit https://github.com/pytorch/pytorch/blob/b9f81a483a7879cd3709fd26bcec5f1ee33577e6/aten/src/ATen/native/Sorting.cpp#L291
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.
Is there a reason you do this check for max input size for quantile and random sample here and not in the forward method of the torch_model.py ?
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.
the normalization parameters are calculated before the actual training
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.
Ok, maybe you can use 2**24 here and reference the line in pytorch that causes the limit
Done
@@ -308,6 +308,8 @@ def forward(self, batch: Tensor, batch_imagenet: Tensor = None) -> Tensor | dict | |||
(ae_output - student_output[:, self.teacher_out_channels :]) ** 2, dim=1, keepdim=True | |||
) | |||
|
|||
map_st = F.pad(map_st, (4, 4, 4, 4)) |
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 don't find this line in the paper..
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.
Without this padding the segmentation metric will decrease very much because the segmentation pixels are not aligned anymore after the next line (map_st = F.interpolate(map_st, size=(self.input_size[0], self.input_size[1]), mode="bilinear")
).
It is similar to applying 11x11 edge filter to 100x100 image. The result will be 90x90 image with detected edges. Resizing it to 100x100 would distort the detected edges. Instead you have to pad the 90x90 image with 5 pixels on each size.
In the case of EfficientAD, the patch size is 33, so 16 pixels padding on each border. The teacher has two layers with stride 2, so we need to divide the 16 pixels padding by 2 and again by 2, which gives us 4. So 4 pixels padding on each border for the output feature maps.
At least your new results on the bottle class however do not match my results on the bottle class with the medium model and the previous version: https://api.wandb.ai/links/alexried/mnft4f0b Regarding (convolutional) padding, thats what the authors say
I guess they use padding for their performance measures but remove it for measuring timing.. |
The authors say: "We time EfficientAD without padding and therefore report the anomaly detection results for this setting in the experimental results of this paper." So I think they use the "without padding" setting for the experimental results of the paper. In my experiments, without padding also works slightly better than with padding. |
@samet-akcay @ashwinvaidya17 @djdameln can you please review the changes? |
Ok alright! Did you train your teacher models with or without padding? |
Co-authored-by: Declan <[email protected]>
Co-authored-by: Declan <[email protected]>
@nelson1425 I have one more question, do you report your accuracy results for the last training epoch or for the best training epoch? |
I have realized another thing regarding the pretrained teachers. Unfortunately I cannot test on all the MVTec categories, but in some of them, the pretrained model by rximg https://github.com/rximg/EfficientAD/tree/main perform better than my models that are currently provided. Maybe you can get results using these models? You just have to replace the models in |
I fully agree that using Ultimately, the problem here is that the MVTec dataset does not have a pre-supplied validation set. For the current problem, the best solution would be to sub-sample the normal images from the training set and use those to compute the quantiles. However, there are some technical issues which prevent us from adding a For now, I suggest we go with @nelson1425's proposed solution. |
@alexriedel1 Do you think the performance of the anomalib model would improve if we switch to Torchvision transforms? We are considering to replace albumentations with torchvision, mainly for convenience reasons. Improved model results would be another reason to prioritize this effort. |
I guess that would make sense, as both timm and torchvision pre-trained models are trained using torchvision transforms. For feature extracting anomaly detection methods using torchvision transforms would bring a performance boost. (Even for the imagenet normalization albumentation and torchvision show slightly different results...) |
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, I'm happy with the changes, though I'm interested in this as well:
@nelson1425 I have one more question, do you report your accuracy results for the last training epoch or for the best training epoch?
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 for the efforts! Looks good to me
@alexriedel1, so you are in favour of using torchvision transforms as well? As @djdameln pointed out, we would like to switch to |
yes right, with their support for bounding boxes and segmentations masks I would also be happy to see the switch to torchvision :) |
I report them for the last training epoch |
I report them for the last training epoch, so after the training and the map normalization are finished. I have added one commit. From my side, the pull request can be merged now. |
Done. Sorry for the delay! @samet-akcay I think your approve is required before the pull request can be merged, correct? |
@ashwinvaidya17 and @djdameln already approved it, so we could merge it after the tests. |
@d3tk, there are torch and lightning model implementations.
|
@samet-akcay I believe I had made a mistake applying the newest changes and so I got import errors. It works for me now once I corrected that. Apologies! |
@samet-akcay The check failed with "FAIL Required test coverage of 70% not reached. Total coverage: 61.03%" |
@samet-akcay how should we proceed? |
@ashwinvaidya17, thoughs about the coverage? Should we reduce the coverage to 60 for now, and address later on? |
I think that EfficientAD could also be added to pre_merge tests for models. I'm not that familiar with entire test infrastructure, but I do believe that would increase coverage? |
oh wait, I think the failure was due to some other CI problem. the coverage is 73% now, and the tests are passing. I'm happy to merge the PR |
@blaz-r, yeah, you are right. It's just a one liner. My concern though is that we need to ensure that we quickly run it for a single forward-pass/iteration or epoch for a quick integration test. Otherwise, the model will increase the run time. We are working on changing the entire test infrastructure as well to address these :) |
@nelson1425, thanks a lot for your effort! Much appreciated! |
Description
I have run EfficientAD-S on the current main branch. It reached 0.972 Image AUROC on average on MVTec AD. The paper's result is 0.988. I have looked at the paper hyperparameters and have updated the settings in anomalib.
Now EfficientAD-S scores 0.982 Image AUROC on average on MVTec AD. These are the changes in anomalib:
I changed the default to EfficientAD-S, because it is faster to train and performs a little bit better than EfficientAD-M in anomalib.
The diff of the markdown files is caused by the pre-commit markdown formatter.
Changes
Checklist