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

Port remaining transforms tests #7954

Merged
merged 26 commits into from
Sep 26, 2023
Merged

Port remaining transforms tests #7954

merged 26 commits into from
Sep 26, 2023

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Sep 11, 2023 •
edited by pytorch-bot bot
Loading

I've tried to make the individual commits as atomic as possible. Meaning, you can probably review the commits one by one.

cc @vfdev-5

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 11, 2023 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/7954

Note: Links to docs will display an error until the docs builds have been completed.

⏳ No Failures, 1 Pending

As of commit 7902956 with merge base 997384c (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pmeier pmeier marked this pull request as ready for review September 13, 2023 07:44
@pmeier pmeier requested a review from NicolasHug September 13, 2023 07:44
@pmeier
Copy link
Collaborator Author

pmeier commented Sep 13, 2023

As of 775e5ec, the old KernelInfo / DispatcherInfo architecture is dead and all tests that depend on it are removed. Since these parts were strongly tied to the functional tests, there are still some legacy transforms test to port. I'll get to that in a separate PR, since this one is already large enough as is.

@pmeier pmeier mentioned this pull request Sep 14, 2023
Merged
@pmeier pmeier mentioned this pull request Sep 25, 2023
Merged
NicolasHug
NicolasHug reviewed Sep 25, 2023
View reviewed changes
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks @pmeier, made a first pass on the the first half of the tests

test/test_transforms_v2_refactored.py Outdated Show resolved Hide resolved
test/test_transforms_v2_refactored.py Outdated Show resolved Hide resolved
test/test_transforms_v2_refactored.py Outdated Show resolved Hide resolved
test/test_transforms_v2_refactored.py
self._assert_is_standard_normal_distributed(output)


class TestClampBoundingBoxes:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we'll want a sanity check for this one (that the bbox is indeed clamped). But I guess this can come in another PR

test/test_transforms_v2_refactored.py Outdated Show resolved Hide resolved
NicolasHug
NicolasHug approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks @pmeier, some last comments but LGTM

test/test_transforms_v2_refactored.py Outdated Show resolved Hide resolved
test/test_transforms_v2_refactored.py Show resolved Hide resolved
test/test_transforms_v2_refactored.py Show resolved Hide resolved
test/test_transforms_v2_refactored.py Outdated Show resolved Hide resolved
test/test_transforms_v2_refactored.py Outdated Show resolved Hide resolved
test/test_transforms_v2_refactored.py Outdated Show resolved Hide resolved
@pmeier pmeier merged commit 1a9ff0d into pytorch:main Sep 26, 2023
54 of 62 checks passed
@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

@pmeier pmeier deleted the port branch September 26, 2023 11:18
facebook-github-bot pushed a commit that referenced this pull request Oct 30, 2023
@NicolasHug @facebook-github-bot
[fbsync] Port remaining transforms tests (#7954)
8730285 
Reviewed By: vmoens

Differential Revision: D50789097

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

@NicolasHug NicolasHug NicolasHug approved these changes

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

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