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.

Sign up for GitHub

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

Jump to bottom

Make RandomIoUCrop compatible with SanitizeBoundingBoxes #7268

Merged
merged 7 commits into from
Feb 16, 2023
Merged

Make RandomIoUCrop compatible with SanitizeBoundingBoxes #7268

merged 7 commits into from
Feb 16, 2023

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Feb 16, 2023 •
edited by pytorch-bot bot
Loading

@NicolasHug NicolasHug requested a review from pmeier February 16, 2023 13:43
NicolasHug
NicolasHug commented Feb 16, 2023
View reviewed changes
test/test_prototype_transforms.py
# set above (which is fine).
(True, "ssd"): 4,
(True, "ssdlite"): 3,
}.get((sanitize, data_augmentation), num_boxes)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I was reviewing that code, I'd probably swear a bit (I can refactor if needed)

pmeier
pmeier reviewed Feb 16, 2023
View reviewed changes
Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit inline. Otherwise, could you also address

vision/test/test_prototype_transforms_consistency.py

Lines 1093 to 1099 in 3991ab9

# FIXME: make
# v2_transforms.Compose([
# v2_transforms.RandomIoUCrop(),
# v2_transforms.SanitizeBoundingBoxes()
# ])
# work
# (det_transforms.RandomIoUCrop(), v2_transforms.RandomIoUCrop(), {"with_mask": False}),

to make sure we actually have the same behavior as we currently have on our references?

torchvision/transforms/v2/_misc.py Show resolved Hide resolved
pmeier
pmeier reviewed Feb 16, 2023
View reviewed changes
test/test_prototype_transforms.py Show resolved Hide resolved
test/test_prototype_transforms.py Show resolved Hide resolved
torchvision/transforms/v2/_geometry.py Show resolved Hide resolved
test/test_prototype_transforms.py Outdated Show resolved Hide resolved
@NicolasHug
Copy link
Member Author

The recent changes you made LGTM @pmeier - thanks a lot for catching the bugs and for the fixes. Happy to merge on my side (can't approve myself)

@NicolasHug NicolasHug mentioned this pull request Feb 16, 2023
Closed
49 tasks
vfdev-5
vfdev-5 approved these changes Feb 16, 2023
View reviewed changes
Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stamp :)

@pmeier pmeier merged commit b7892d3 into pytorch:main Feb 16, 2023
@pmeier pmeier deleted the sanitize_and_ioucrop branch February 16, 2023 17:54
@github-actions
Copy link

Hey @pmeier!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

facebook-github-bot pushed a commit that referenced this pull request Mar 28, 2023
@NicolasHug @facebook-github-bot
[fbsync] Make RandomIoUCrop compatible with SanitizeBoundingBoxes (#7268
1b000ff 
)

Summary: Co-authored-by: Philip Meier <[email protected]>

Reviewed By: vmoens

Differential Revision: D44416643

fbshipit-source-id: b2e1ec5f1a9fa1edc2bb5a459606bcf30c9dbeb0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vfdev-5 vfdev-5 vfdev-5 approved these changes

@pmeier pmeier pmeier approved these changes

Assignees
No one assigned
Labels
cla signed module: transforms prototype
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@NicolasHug @vfdev-5 @pmeier @facebook-github-bot