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

Add manual opcheck tests for roi ops #8144

Merged
merged 3 commits into from Dec 5, 2023

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Dec 5, 2023

Closes #8055 and specitically addresses #8055 (comment)

cc @pmeier

Copy link

pytorch-bot bot commented Dec 5, 2023

🔗 Helpful Links

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

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

✅ You can merge normally! (19 Unrelated Failures)

As of commit a473808 with merge base fe0d280 (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

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

}
}
}
"data": {}
Copy link
Member Author

Choose a reason for hiding this comment

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

This file still needs to exist even if it's empty, unfortunately.

@@ -610,15 +610,6 @@ def test_jit_boxes_list(self):
self._helper_jit_boxes_list(model)


optests.generate_opcheck_tests(
testcase=TestRoIAlign,
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the roi_align opcheck tests to the function below

@@ -676,6 +667,43 @@ def test_boxes_shape(self):
self._helper_boxes_shape(ops.ps_roi_align)


@pytest.mark.parametrize(
Copy link
Member Author

Choose a reason for hiding this comment

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

This could be in RoIOpTester as a base test, but I personally find it really hard to figure out what is being tested with such inheritance structure. So I'm opting for a self-contained parametrized test here.

(The test inheritance structure predates the adoption of pytest.)

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.

Thanks!

@NicolasHug NicolasHug merged commit 1ababf9 into pytorch:main Dec 5, 2023
26 of 44 checks passed
@NicolasHug NicolasHug deleted the lajenflajenflajenf branch December 5, 2023 13:21
facebook-github-bot pushed a commit that referenced this pull request Jan 16, 2024
Reviewed By: vmoens

Differential Revision: D52539014

fbshipit-source-id: 6fc2eb107216f1c3933e199a75dc51e620d7c093
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.

Add torch.compile support for all ops
3 participants