PERFORMANCE IMPROVEMENT

[mjack3]

Inference speed highly increased from 6s to 2s for the 5 images used in the notebook. Now also support CUDA and speed is increased to 0,02s using a RTX 3090

Based on your sample images from notebook

# load test Images
DATASET_PATH = os.path.realpath("../../datasets/MVTec")
paths = [
    os.path.join(DATASET_PATH, "bottle/test/broken_large/000.png"),
    os.path.join(DATASET_PATH, "bottle/test/broken_small/000.png"),
    os.path.join(DATASET_PATH, "bottle/test/contamination/000.png"),
    os.path.join(DATASET_PATH, "bottle/test/good/000.png"),
    os.path.join(DATASET_PATH, "bottle/test/good/001.png"),

I meassured the total time making predicitons

print('Making predictions')
tik = time.time()
image_scores, score_maps = patch_core.predict(batch) # Make prediction
tok = time.time()
print("Done")
print(tok - tik)

This is the result

Now the performance have been highly increasd by CPU

And now neighbour search support supports GPU

[mjack3]

Yes i am getting the similar results as in your patchcofre_example notebook

[mjack3]

I added CUDA Support I meassured the time in this way

This are the result before and after

As the distances are calculated in CPU, this is not a huge change but it works bit faster

[antonemanuel]

Hi!

Fun that you want to contribute to the project! Cool that you got such a speed improvement, that would be really nice to include in the repo. I have some things I would like you to fix and some questions before I merge:

1. First of all I would like you to add type hints for parameters in functions you add as done in other functions in the repo. Also I would like you to add docstrings for functions and classes. We use google style docstrings (https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html), but it should be clear from looking at other functions/classes in the repo. If it is very clear what the parameters are and what the function does, a short one line docstring is sufficient.
2. feature_extraction.py: Line 112 in feature extraction: device = self.device seems to me that it doesn't do anything. I guess it should be removed. However I believe line 114 should stay.
3. patchcore.py: You added two classes NN & KNN. Is it necessary to have both and is it necessary to have all those methods? I don't see them being used.
4. patchcore.py: You changed the default argument for backbone to "wide_resnet50". I think it should still be "resnet18".
5. patchcore.py: You added two prints. They should be removed.
6. patchcore.py: On line 168 you added batch = batch.to(self.device). This should be removed. We have chosen to handle the device separately for model and data for clarity.
7. patchcore.py: You import faiss. I don't see it being used. Is it necessary and if so for what?
8. utils.py: You changed values in standard_image_transform and standard_mask_transform. Why? If no reason I would prefer them to stay as before.
9. utils.py: You removed the device in to_batch. I think this has to do with (5) and I think it should stay as before.

Please say if anything I wrote here seems weird or if you have a reason for something I said to change.

[mjack3]

I will answer as soon as possible :)

[antonemanuel]

Great! I saw you added something more to the pull request now. If it is possible and you implement different features, please do multiple smaller pull requests instead of one large.

[mjack3]

Hello.

1. The parameters are clear, i think is not need to add complicated documentation about them. I added style docstrings documented in the the function :)
2. You are right. Now it is fixed
3. It is not necessary. Now It is fixed
4. The paper uses wide_resnet50.
5. I thought it was an interesting info, but of course you can remove it.
6. For me is clearest to specify the device when you are creating the model. It's expected that if you want to execute on GPU, all the process should be in the GPU. It 's incoherent to allow running the model in CPU and the batch in GPU. But if for you have sense, it' s ok :)
7. I have been working on integrating FAISS but I am not getting good performance. So it can be deleted.
8. If you follow the guidance of the official paper, it says that images are resized to 256 and then center cropped to 224. I used Lanczos because has good results for downsamling
9. Same as i said in 6. For me batch just should to create a Tensor Batch. In this way this is so clear as

For train

For inference

I think that this general idea of changing to_batch will affect to PADIM implementation, but it is straight forward to change it. If you dont like any idea, please, feel you free to use what you consider from my commit :)

PDT: my last commit just fixes little mistakes and nothing new was implemented