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 RandomCrop transform #69

Merged
merged 3 commits into from Jun 22, 2022
Merged

Fix RandomCrop transform #69

merged 3 commits into from Jun 22, 2022

Conversation

donlzx
Copy link
Contributor

@donlzx donlzx commented Jun 12, 2022

Fix RandomCrop transform missing one offset position, use round() instead of floor() for startindices.

Fix RandomCrop transform msssing one offset position, use round() instead of floor() for startindices.
@donlzx
Copy link
Contributor Author

donlzx commented Jun 12, 2022

I have verified the changes with the MNIST and CIFAR-10/100 datasets. Other datasets with bigger images should not be affected, but I haven't tested with these datasets.

@lorenzoh
Copy link
Member

Thanks for the PR!

Can you give a short explanation for the latest change? And do you have some before/after images?

@donlzx
Copy link
Contributor Author

donlzx commented Jun 17, 2022

The first commit use round() instead of floor() to fix the problem of missing one shift position when cropping with padding. However, it was done in a way different from torchvision, which can yield better or worse performance depending on the padding size.

The second commit instead fixes the issue by generating the random shift positions the same way as in torchvision. I will run more experiments to verify that the performance is identical to using PyTorch+torchvision.

@donlzx
Copy link
Contributor Author

donlzx commented Jun 18, 2022

Here are 20 images generated using RandomCrop with DataAugmentation v0.2.8. The original images (32x32 in size) are padded by 2 pixels on each edge, randomly cropped to 32x32, and then up-sampled to 128x128 for display.

The left and top edges of the images are randomly padded by either 0/1/2 pixels, while the right edges are only paded by 0/1 pixels.

randcrop_before

@donlzx
Copy link
Contributor Author

donlzx commented Jun 18, 2022

The following are 20 images generated with the latest fixes from this pull request . All the edges of the images are randomly padded by 0/1/2 pixels.

randcrop_after

@lorenzoh
Copy link
Member

I did some investigation regarding the test failure and I think I've found the issue. When calculating the new bounds after cropping and there is some wiggle room (i.e. the image is larger than the crop region), the RandomCrops random state has a number from 0. to 1. for every dimension. If the number is exactly one, the rounding logic breaks, so we need to make sure it is just below 1.

src/projective/crop.jl Outdated Show resolved Hide resolved
@lorenzoh
Copy link
Member

lorenzoh commented Jun 22, 2022

Thanks a bunch for taking the time to find, investigate and fix! New version is tagged 👍

@lorenzoh lorenzoh merged commit 0cc7be4 into FluxML:master Jun 22, 2022
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.

None yet

2 participants