-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Fix aspect ratio sampling for RandomResizedCrop #14585
Conversation
@zhreshold @szha Could you have a look? @mxnet-label-bot add [pr-awaiting-review] |
Thanks for your contribution, would you mind adding a test case for this situation? |
@marcoabreu added test in test_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.
LGTM
@abhinavs95 Can you look into the CI-failures ? |
python/mxnet/image/image.py
Outdated
@@ -585,14 +586,12 @@ def random_size_crop(src, size, area, ratio, interp=2, **kwargs): | |||
area = (area, 1.0) | |||
for _ in range(10): | |||
target_area = random.uniform(area[0], area[1]) * src_area | |||
new_ratio = random.uniform(*ratio) | |||
log_ratio = (math.log(ratio[0]), math.log(ratio[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.
you don't need to import math
, use np
which is already imported
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.
done!
@szha could you have a look and merge? Thanks @mxnet-label-bot update [pr-awaiting-merge] |
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 distribution is centered at (ratio[1] - ratio[0]) / (log(ratio[1]) - log(ratio[0])), rather than 1.
Thank you for the fix!
LGTM.
* added log sampling for aspect ratio * added test * added comments * fix test * remove math, fix test
Description
Fixes #14505
Aspect ratio goes out of bounds when the bounds are not reciprocals. Fixed by sampling from log scale instead of linear scale. When sampling from log scale, random swapping of width/height is not required since the distribution is centered at 1.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.