-
Notifications
You must be signed in to change notification settings - Fork 709
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 support for non-square images #204
Conversation
@@ -40,13 +42,14 @@ def __init__( | |||
): | |||
super().__init__() | |||
|
|||
assert input_size % 16 == 0, "Input size should be a multiple of 16" | |||
assert input_size[0] % 16 == 0 and input_size[1] % 16 == 0, "Input size should be a multiple of 16" |
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.
I guess this is no longer needed now that we use padding to make sure that the network can process any image size.
""" | ||
# find the largest dimension | ||
l_dim = 2 ** math.ceil(math.log(max(*input_size), 2)) | ||
padding = math.ceil((l_dim - input_size[0]) // 2 + 1), math.ceil((l_dim - input_size[1]) // 2 + 1) |
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.
This won't work for odd image size, e.g. 221
return torch.mean(torch.pow((latent_i - latent_o), 2), dim=1).view(-1) # convert nx1x1 to n | ||
|
||
def get_padded_tensor(self, batch: Tensor) -> Tensor: |
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.
Minor issue, but I don't like this name. Maybe apply_padding
or pad_inputs
? I also think the method could be static.
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.
I'll update it in a bit. Static is a good idea. In that case would it make sense to move it to data/utils/image.py
?
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.
Yes, I think it would make sense to move it to a shared location so that other modules could access it as well. In that case we should maybe name it pad_nextpow2
or similar
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.
Thanks! Just one minor comment left
anomalib/data/utils/image.py
Outdated
"""Compute required padding from input size and return padded images. | ||
|
||
Finds the largest dimension and computes a square image to pass into the model. In case the image dimension |
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.
Maybe mention in the description that the function pads the images so that both sides are a power of 2.
Description
Computes padding based on the image size. Might not be the best approach.
Fixes GANomaly breaks when changing image size #136
Changes
Checklist