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.

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

Split tests for transforms v2 and prototype #7278

Merged
merged 15 commits into from
Feb 17, 2023
Merged

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Feb 17, 2023

With the branch cut coming around the torchvision/prototype namespace will be removed for the release branch. Right now all the tests for the migrated torchvision.datapoints and torchvision.transforms.v2 unconditionally import from torchvision.prototype and thus will fail. This PR does the following:

  • Move all v2 functionalities from prototype_common_utils into common_utils. The only things remain in prototype_common_utils is the label and one hot label data generation
  • Create test_datapoints and move everything non-label related there from test_prototype_datapoints
  • Rename prototype_transforms_{kernel,dispatcher}_infos and test_prototype_transforms_{functional,utils} into *transforms_v2*
  • create a new test_transforms_v2 and move most of test_prototype_transforms there. Some things like batch or video transforms stay in test_prototype_transforms
  • Rename test_prototype_transforms_consistency to test_transforms_v2_consistency and remove all label related stuff. Furthermore, move test for FixedSizedCrop to test_prototype_transforms.

cc @vfdev-5 @bjuncek

@@ -170,23 +170,6 @@ def wrapper(self):
return wrapper


def combinations_grid(**kwargs):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've moved this to common_utils since it is no longer just used by the dataset tests.

test/test_transforms_v2_consistency.py Outdated Show resolved Hide resolved
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 Philip, obviously I can't review individual files, but the main lines above LGTM. One minor Q is about

Create test_datapoints and move everything non-label related there from test_prototype_transforms

Why move transforms-related tests into test_datapoints? Is there a typo?

@pmeier
Copy link
Collaborator Author

pmeier commented Feb 17, 2023

Why move transforms-related tests into test_datapoints? Is there a typo?

Yes. It should say

Create test_datapoints and move everything non-label related there from test_prototype_datapoints

Will update my top comment.

@pmeier
Copy link
Collaborator Author

pmeier commented Feb 17, 2023

Except for Python 3.8, all unittest jobs on macOS are failing due to insufficient resources on the runner: https://app.circleci.com/pipelines/github/pytorch/vision/23382/workflows/05e31fc3-0271-4ad4-8d48-a0b82b3e099f/jobs/1834265?invite=true#step-109-223.

We are using

resource_class: large

which translates to

8 @ 2.7 GHz | 16GB

There is also macos.x86.metal.gen1

12 @ 3.2 GHz | 32GB

Will try this one.

@pmeier
Copy link
Collaborator Author

pmeier commented Feb 17, 2023

Unittest on Windows and Linux are green though, so I'' revert 67fdc6d.

@NicolasHug NicolasHug mentioned this pull request Feb 17, 2023
49 tasks
@pmeier
Copy link
Collaborator Author

pmeier commented Feb 17, 2023

It seems we don't have access to the larger runner: https://app.circleci.com/pipelines/github/pytorch/vision/23385/workflows/979cac8c-f706-44cc-8a7b-8102a58ebbe4/jobs/1834346

Job was rejected because resource class macos.x86.metal.gen1, image xcode:14.0 is not a valid resource class

@pmeier pmeier merged commit 4774fe3 into pytorch:main Feb 17, 2023
@pmeier pmeier deleted the split-v2-tests branch February 17, 2023 14:17
facebook-github-bot pushed a commit that referenced this pull request Mar 29, 2023
Reviewed By: vmoens

Differential Revision: D44416542

fbshipit-source-id: 7ed18b0218f1b65aa1f7b016f56777e55207ac53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants