Skip to content

Implementation of GLASS model in Anomalib #2629

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

code-dev05
Copy link

@code-dev05 code-dev05 commented Mar 26, 2025 β€’

πŸ“ Description

✨ Changes

Select what type of change your PR is:

  • πŸš€ New feature (non-breaking change which adds functionality)
  • πŸ“š Documentation update

TODO

  • Implement the model from this research paper.
  • Write a trainer for the above model.
  • Add comments to the code for easier understanding.
  • Write test cases for verification.
  • Update documentation for the above model.
  • Any optimizations if needed?

I will keep updating the PR as I implement the rest of the list.

Signed-off-by: Devansh Agarwal <devansh.agarwal.official@gmail.com>
@code-dev05 code-dev05 force-pushed the feature/model/glass branch from 674ebd8 to 5b4931b Compare March 26, 2025 11:57
@code-dev05
Copy link
Author

@samet-akcay I am facing a problem. I need to add perlin noise to the image for training as done here in the getitem method.
Should i add it to the image during the training step or should I make a dict before the training step as in the above link?
For the second way, I think I would need to make a new dataset loader.

@samet-akcay
Copy link
Contributor

samet-akcay commented Apr 12, 2025 β€’

@code-dev05, you could use Anomalib's PreProcessor class. The advantage of the preprocessor is that you could apply any transform to the model input, including perlin noise. You could set train,val, test transforms separately, or all at once.

Anomalib also provides PerlinAnomalyGenerator class that you could create as a transform and pass into PreProcessor

Let me know if the documentation is not clear, or you would need more clarification.

@code-dev05
Copy link
Author

Thank you for the advice. I will look into it and get back to you if I do not understand something.

@samet-akcay
Copy link
Contributor

Alternatively, you could use perlin_noise_generator function in on_train_batch_start method in your Glass implementation that inherits AnomalibModule

What this would do is to apply the perlin noise when the model receives the input on batch start.

You could prototype any one of them. We could always polish it later on once your pr is ready for review

Signed-off-by: Devansh Agarwal <devansh.agarwal.official@gmail.com>
@code-dev05
Copy link
Author

@samet-akcay I have created one new commit. Please check it. I have noticed a few issues,

  • One is computing the center of the dataset. Since we are using the Batch dataclass, in one training step we only compute the outputs for one batch, and for the center we need the whole dataset. One solution for this would be to somehow calculate the center for the whole dataset during initialisation and other way would be to calculate the center of the batch for that training step. This approach would be more inaccurate than the other.
  • Second is the the use of two different datasets. In the original code, they have used DTD dataset along with MVTec for different textures. How to implement that? In my code I have used batch.image and batch.aug so I am assuming we are storing both the image and the texture in the object. I think we have to make a new dataclass for this? If so, we can directly integrate the perlin noise in the image instead of during training step.
    Thank you for answering my queries.

@code-dev05
Copy link
Author

@samet-akcay I will also need to create a datamodule for the DTD dataset. Should I add that in this PR only or create a new PR?

@@ -0,0 +1 @@
from .lightning_model import Glass
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you install pre-comit hooks and run it again? It will sort out such issues

Copy link
Author

Choose a reason for hiding this comment

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

the pre-commit hook is suggesting to change the line to from .lightning_model import Glass as Glass . Is this correct?

@@ -0,0 +1,50 @@
import torchvision.models as models
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to attribute files like these to the original implementation. Also, include an entry in third-party.txt
https://github.com/open-edge-platform/anomalib/blob/main/third-party-programs.txt

Comment on lines +20 to +24
self,
backbone,
input_shape,
pretrain_embed_dim,
target_embed_dim,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you type these as well? Same for the other files.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also please provide default values. Users should be able to instantiate the model without passing any args.

Copy link
Author

Choose a reason for hiding this comment

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

should i put a default value for input_shape also? I think this one should be left for the user.

@@ -0,0 +1,161 @@
import torch
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add license and docstring to the top of all the files?

from .loss import FocalLoss
from .torch_model import GlassModel

class Glass(AnomalibModule):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please list your new model class in src/anomalib/models/image/__init__.py and src/anomalib/models/__init__.py, to enable importing with from anomalib.models import Glass.

pre_proj: int = 1,
dsc_layers: int = 2,
dsc_hidden: int = 1024,
dsc_margin: int = 0.5,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dsc_margin: int = 0.5,
dsc_margin: float = 0.5,

noise: float = 0.015,
radius: float = 0.75,
p: float = 0.5,
lr: int = 0.0001,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
lr: int = 0.0001,
lr: float = 0.0001,

Comment on lines +20 to +24
self,
backbone,
input_shape,
pretrain_embed_dim,
target_embed_dim,
Copy link
Contributor

Choose a reason for hiding this comment

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

Also please provide default values. Users should be able to instantiate the model without passing any args.

from .loss import FocalLoss
from .torch_model import GlassModel

class Glass(AnomalibModule):
Copy link
Contributor

Choose a reason for hiding this comment

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

The class is missing the following mandatory properties: learning_type and trainer_arguments. Without the implementation of these properties, it won't run. You can have a look at the other models for example implementations.

@djdameln
Copy link
Contributor

djdameln commented Apr 23, 2025 β€’

Hi @code-dev05

Thanks a lot for your effort! Here are some comments that may help you address your issues related to center computation and noise generation:

  • One is computing the center of the dataset. Since we are using the Batch dataclass, in one training step we only compute the outputs for one batch, and for the center we need the whole dataset. One solution for this would be to somehow calculate the center for the whole dataset during initialisation and other way would be to calculate the center of the batch for that training step. This approach would be more inaccurate than the other.

We want to reproduce the algorithm from the original paper as closely as possible, so I would suggest to compute the center of the dataset in an initialization step. You could try using the setup or on_fit_start/on_train_start lightning hooks for this (see lightning docs for further reading on model hooks).

  • Second is the the use of two different datasets. In the original code, they have used DTD dataset along with MVTec for different textures. How to implement that? In my code I have used batch.image and batch.aug so I am assuming we are storing both the image and the texture in the object. I think we have to make a new dataclass for this? If so, we can directly integrate the perlin noise in the image instead of during training step.

Looking at your code, it is not entirely clear to me how you are intending to use batch.aug. The current (Image)-Batch class does not contain the aug key. Were you planning on adding it? This would involve fundamental changes to Anomalib's dataset and dataclasses interface, which I'm not sure is the way to go here.

My personal preference would be to apply the augmentation within the training_step. This is similar to how Perlin noise augmentations are used in DRAEM model (see here). In my view, this is the most flexible approach, as the training step retains access to both the original and the augmented image. At the same time, we ensure that the augmentation is always applied before passing the images to the model during training (contrary to adding the noise as an augmentation step on the dataset side, which could lead to problems if the dataset is not configured correctly by the user).

I will also need to create a datamodule for the DTD dataset. Should I add that in this PR only or create a new PR?

This might not be necessary. As far as I understand (but please correct me if I'm wrong), we only use the DTD dataset as a source of textures for the synthetic anomaly generation step. This means that we don't need any ground truth information for this dataset, just the raw images. So instead of implementing a dedicated dataset/datamodule, we can just randomly sample some images that we read from the file system in every step. In fact, this mechanism has already been implemented for DRAEM, which uses a similar approach. Please have a look at this class to see if you can use this for the augmentation step of your GLASS model (note that you can just pass the root of the DTD dataset on your file system, and the augmenter will automatically sample and read the images).

@code-dev05
Copy link
Author

Looking at your code, it is not entirely clear to me how you are intending to use batch.aug. The current (Image)-Batch class does not contain the aug key. Were you planning on adding it? This would involve fundamental changes to Anomalib's dataset and dataclasses interface, which I'm not sure is the way to go here.

Yes, I was planning on adding a new dataclass interface but i will see the way it is done in the DRAEM model and try to get the same done here and will add the usage of the hooks for computing the center as soon as possible.

I will also make the suggested changes as soon as possible.

Thanks for the help.

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.

πŸ“– [STORY] Implementing the GLASS Anomaly Detection Model in Anomalib
4 participants