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

Potential bug when creating image crops #5

Closed
ManicMunchkin opened this issue Oct 30, 2023 · 4 comments
Closed

Potential bug when creating image crops #5

ManicMunchkin opened this issue Oct 30, 2023 · 4 comments

Comments

@ManicMunchkin
Copy link

Hi,
first off I want to thank you for publishing an actually working codebase and well documented project.
I am using Eq-Transporter Nets in my Master Thesis and when looking through the code in the Attention and Transport modules I noticed a potential bug when crops are taken from the input images.

e.g. Line 121 from the attention module: crop = input_tensor[:,:,p[0]:(p[0] + self.crop_size),p[1]:(p[1] + self.crop_size)]
or Line 89 from the transport module: crop = crop[:, :, p[0]:(p[0] + self.crop_size_1), p[1]:(p[1] + self.crop_size_1)]

Wouldn't this put the upper left corner of the crop window at the given location instead of centering the crop window at the location?
I would expect the crop to be made like this:
crop = input_tensor[:,:,(p[0]-crop_size/2):(p[0]+crop_size/2),(p[1]-crop_size/2):(p[1]+crop_size/2)]
So that it is centered at the pick location.

Is this an actual bug or am I just missing something here?

@HaojHuang
Copy link
Owner

Hi, the image was padded.

@ManicMunchkin
Copy link
Author

Hi,
true, however it is padded by crop_size/2 in every direction (up, left, down, right) in the Attention and the Transport network.
e.g.

self.pad_size_1 = int(self.crop_size_1 / 2)
self.padding_1 = np.zeros((3, 2), dtype=int)
self.padding_1[:2, :] = self.pad_size_1

Which actually illustrates my point, because padding by crop_size/2 would make sense if the crop is centered around p, since then the maximum overlap over the input image boundaries would be crop_size/2. This would be figure B in the following drawing.

Figure A shows the way cropping is implemented as I understood it, which could lead to overlap over the padded area.
Figure B shows the cropping centered around the pixel p.
image

@HaojHuang
Copy link
Owner

Hi,

The coordinate of P changes after padding since the reference frame changed. For example, the coordinate of the upper left corner of the original image is (0,0) and after padding with 10 pixels on the four sides of the image, the coordinate of the upper left corner of the original image is (10, 10) instead of (0,0).

@ManicMunchkin
Copy link
Author

Aah that makes sense.
Thanks for the explanation!

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

No branches or pull requests

2 participants