Optimized anomaly score calculation for PatchCore for both num_neighb…

[VdLMV]

Description

• Optimized, for inference time, anomaly score calculation for PatchCore. Small gain if num_neighbors >1 by using min operator instead of topk(1). Larger gain if num_neighbors == 1 because weight calculation is not required.

Changes

• Bug fix (non-breaking change which fixes an issue)
• Refactor (non-breaking change which refactors the code base)
• 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

• [ x] My code follows the pre-commit style and check guidelines of this project.
• [ x] I have performed a self-review of my code
• [x ] I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• [ x] My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• [x ] New and existing tests pass locally with my changes

[samet-akcay]

@VdLMV can you please provide a little bit more description?

[VdLMV]

@samet-akcay It looks like the original title of PR was too long and is broken in two parts.
More elaborate description: Optimized, for inference time, anomaly score calculation for PatchCore. Small gain if num_neighbors >1 by using min operator instead of topk(1). Larger gain if num_neighbors == 1 because weight calculation is not required.

[djdameln]

Thanks for spotting this! In addition to speeding up the computation, it turns out that your suggestion also fixes a bug in the patchcore model which caused the forward pass to produce an error when both test_batch_size and num_neighbors were 1.

I do feel that the implementation could be made a bit cleaner to improve readability of the code. I would suggest to move the logic related to n_neighbors == 1 out of the forward method and into self.nearest_neighbors and self.compute_anomaly_score.

in self.nearest_neighbors:

distances = torch.cdist(embedding, self.memory_bank, p=2.0)
if n_neighbors == 1:
    # when n_neighbors is 1, speed up computation by using min instead of topk
    patch_scores, locations = distances.min(1)
else:
    patch_scores, locations = distances.topk(k=n_neighbors, largest=False, dim=1)
return patch_scores, locations

and at the top of self.compute_anomaly_score:

# Don't need to compute weights if num_neighbors is 1
if self.num_neighbors == 1:
    return patch_scores.amax(1)

The forward method would then remain unchanged which improves the understandability of the data flow in the model.

[samet-akcay]

@VdLMV, thanks again for creating this PR and contributing to the library!

I agree with @djdameln regarding his comments to improve the readability of the code. If you could address the comments, we would be able to merge it to main. Thanks!

[VdLMV]

I have addressed the comments.
In compute_anomaly_score I added dimension to softmax to prevent warning implicit dimension choice for softmax has been deprecated.

[djdameln]

Thanks. I'm almost happy with the changes. Just one minor comment left

[djdameln]

these lines are duplicate and can be removed

[VdLMV]

@djdameln sorry I overlooked this.

[djdameln]

Thanks!

[samet-akcay]

@VdLMV, the CI unfortunately fails because of a formatting issue. Since it failed, the tests didn't run neither.

Will you be able to run black -l 120 anomalib and push your changes so we could run the tests? Thanks!