-
Notifications
You must be signed in to change notification settings - Fork 734
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
base: main
Are you sure you want to change the base?
Implementation of GLASS model in Anomalib #2629
Conversation
Signed-off-by: Devansh Agarwal <devansh.agarwal.official@gmail.com>
674ebd8
to
5b4931b
Compare
@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. |
@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 Let me know if the documentation is not clear, or you would need more clarification. |
Thank you for the advice. I will look into it and get back to you if I do not understand something. |
Alternatively, you could use perlin_noise_generator function in 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>
@samet-akcay I have created one new commit. Please check it. I have noticed a few issues,
|
@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 |
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.
Can you install pre-comit
hooks and run it again? It will sort out such issues
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 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 |
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 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
self, | ||
backbone, | ||
input_shape, | ||
pretrain_embed_dim, | ||
target_embed_dim, |
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.
Can you type these as well? Same for the other files.
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.
Also please provide default values. Users should be able to instantiate the model without passing any args.
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.
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 |
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.
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): |
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.
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, |
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.
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, |
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.
lr: int = 0.0001, | |
lr: float = 0.0001, |
self, | ||
backbone, | ||
input_shape, | ||
pretrain_embed_dim, | ||
target_embed_dim, |
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.
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): |
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 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.
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:
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
Looking at your code, it is not entirely clear to me how you are intending to use My personal preference would be to apply the augmentation within the
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). |
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. |
π Description
β¨ Changes
Select what type of change your PR is:
TODO
I will keep updating the PR as I implement the rest of the list.