-
Notifications
You must be signed in to change notification settings - Fork 382
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
feat: Added ChannelShuffle transformation and fixes RandomCrop #768
Conversation
Codecov Report
@@ Coverage Diff @@
## main #768 +/- ##
==========================================
- Coverage 95.99% 95.97% -0.03%
==========================================
Files 129 129
Lines 4818 4839 +21
==========================================
+ Hits 4625 4644 +19
- Misses 193 195 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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 !
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 for the fix!
Sorry I had to solve some conflicts after the last commits on main 😅 |
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
min(int(round((start_y + crop_h) * h)), h - 1) | ||
) | ||
croped_img, crop_boxes = F.crop_detection(img, target["boxes"], crop_box) | ||
xmin, ymin = random.uniform(0, 1 - crop_w), random.uniform(0, 1 - crop_h) |
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.
Given the range of scale and ratio could it not be that if the maximum value of crop_w = math.sqrt(scale / ratio)
for instance is chosen, it will be more than 1 and we might end up getting a negative value for the argument: random.uniform(0, 1 - crop_w)
?
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, but that's why it's clipped later on!
We clip it at the end to minimize the constraints on the distribution/randomness
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.
Its clear now. Thanks!
Following up on #730, this PR introduces the following modifications:
ChannelShuffle
RandomCrop
which was making the wrong static assumption of channels_last (cf.doctr/doctr/transforms/modules/base.py
Line 177 in f2f1f21
GaussianNoise
fortorch.uint8
(from feat: Added random noise augmentation to object detection #654)The following snippet:
with the following image
produces:
(since it's a PNG, the background, which was transparent, is by default cast to black upon reading the image)
Any feedback is welcome!