Skip to content
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

Fix post-processing visualization issues #2534

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

djdameln
Copy link
Contributor

📝 Description

Fixes visualization issue when dataset has no GT masks. In this case, we need to explicitly use the image threshold as a surrogate value for the pixel threshold. This was previously not accounted for by the post processor.

✨ Changes

  • Adds properties for retrieving image and pixel threshold to post-processor.
  • Pixel threshold property returns image threshold value when pixel threshold is not available.
  • Adds a basic set of unit tests for the OneClassPostProcessor. This may need to be extended in the future to cover full functionality and all edge cases.

Select what type of change your PR is:

  • 🐞 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)
  • 📚 Documentation update
  • 🔒 Security update

✅ Checklist

Before you submit your pull request, please make sure you have completed the following steps:

  • 📋 I have summarized my changes in the CHANGELOG and followed the guidelines for my type of change (skip for minor changes, documentation updates, and test enhancements).
  • 📚 I have made the necessary updates to the documentation (if applicable).
  • 🧪 I have written tests that support my changes and prove that my fix is effective or my feature works (if applicable).

For more information about code review checklists, see the Code Review Checklist.

@alexriedel1
Copy link
Contributor

For claasification tasks the Visualizer will then produce segmentation maps based on the pixel threshold, is this correct?
And because for many models, the image anomaly score is just the max pixel value, this will somewhat work right?

If I understood correctly, this will fail if the image anomaly score is calculated separately from the pixel anomaly score (as in patchcore for example)

@djdameln
Copy link
Contributor Author

For claasification tasks the Visualizer will then produce segmentation maps based on the pixel threshold, is this correct? And because for many models, the image anomaly score is just the max pixel value, this will somewhat work right?

Yes, if the dataset does not contain ground truth masks, we can't compute the pixel threshold adaptively. In this case we use image threshold, because for many models the ranges of image- and pixel-scores are very similar. This means that the value of the adaptive image threshold is usually a good estimate of the value of the pixel threshold if the latter cannot be computed explicitly.

If I understood correctly, this will fail if the image anomaly score is calculated separately from the pixel anomaly score (as in patchcore for example)

If the range of image scores differs widely from the range of pixel scores, like in Patchcore, using the image threshold may not lead to optimal results. However, it's the best we can do given the lack of pixel-level ground truth annotations. It is also not different from how we did it in previous versions. The main motivation for this approach is ease-of-use. Automatically using the image threshold results in reasonable predictions for most models, which may help the average user and which adds to the plug-and-play nature of the library.

An alternative option would be to make this behaviour configurable, and disable it for models with incompatible image- and pixel-scores like Patchcore. In v2, each model is now responsible for defining a default post-processor that should be applied to the raw predictions. We could easily add a flag to the signature of OneClassPostProcessor to disable the auto-image-threshold mechanism in certain models.

@alexriedel1 If you have any other ideas, please feel free to share! As always, your suggestions are more than welcome :)

@alexriedel1
Copy link
Contributor

For claasification tasks the Visualizer will then produce segmentation maps based on the pixel threshold, is this correct? And because for many models, the image anomaly score is just the max pixel value, this will somewhat work right?

Yes, if the dataset does not contain ground truth masks, we can't compute the pixel threshold adaptively. In this case we use image threshold, because for many models the ranges of image- and pixel-scores are very similar. This means that the value of the adaptive image threshold is usually a good estimate of the value of the pixel threshold if the latter cannot be computed explicitly.

If I understood correctly, this will fail if the image anomaly score is calculated separately from the pixel anomaly score (as in patchcore for example)

If the range of image scores differs widely from the range of pixel scores, like in Patchcore, using the image threshold may not lead to optimal results. However, it's the best we can do given the lack of pixel-level ground truth annotations. It is also not different from how we did it in previous versions. The main motivation for this approach is ease-of-use. Automatically using the image threshold results in reasonable predictions for most models, which may help the average user and which adds to the plug-and-play nature of the library.

An alternative option would be to make this behaviour configurable, and disable it for models with incompatible image- and pixel-scores like Patchcore. In v2, each model is now responsible for defining a default post-processor that should be applied to the raw predictions. We could easily add a flag to the signature of OneClassPostProcessor to disable the auto-image-threshold mechanism in certain models.

@alexriedel1 If you have any other ideas, please feel free to share! As always, your suggestions are more than welcome :)

Ok I see! I think it's ok to do it like this for the purpose of simplicity, you're right.
In case people are getting misleading segmentation results in their classification tasks, maybe there can be a hint in the docs that segmentation for classificatio might not always work as expected

Copy link
Collaborator

@ashwinvaidya17 ashwinvaidya17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants