Closed donlzx closed 2 years ago
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.
Thanks for the PR!
Can you give a short explanation for the latest change? And do you have some before/after images?
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.
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.
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.
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 RandomCrop
s 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.
Thanks a bunch for taking the time to find, investigate and fix! New version is tagged 👍
Fix RandomCrop transform missing one offset position, use round() instead of floor() for startindices.