Test parametrization for instantiated device-specific tests #60233
Conversation
💊 CI failures summary and remediationsAs of commit 6ea2ce8 (more details on the Dr. CI page and at hud.pytorch.org/pr/60233):
Preview docs built from this PR This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
jbschlosser
force-pushed
the
test_parameterization
branch
from
224f49a to
2716c29
Compare
jbschlosser
force-pushed
the
test_parameterization
branch
2 times, most recently
from
01ba128 to
a919295
Compare
jbschlosser
changed the title
|
Do you think the XLA failure is related? |
jbschlosser
force-pushed
the
test_parameterization
branch
from
4a745f2 to
51fe816
Compare
jbschlosser
changed the title
There was a problem hiding this comment.
I'm not in love with the string matching but I guess it's OK as long as we document it
There was a problem hiding this comment.
True, I'm not in love with it either.. Alternatively to doing this, the device / devices arg could be unconditionally passed again. It could probably be argued that all tests instantiated with instantiate_device_type_tests() should accept and use device or devices, but unfortunately some tests (probably wrongly) violate this.
There was a problem hiding this comment.
Hey @jbschlosser! This seems like a cool refactor. I made a couple inline comments for your review. Approving this now since I'll be on pto the rest of this week, however.
|
@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
jbschlosser
force-pushed
the
test_parameterization
branch
from
f3409a8 to
6ea2ce8
Compare
|
@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@jbschlosser merged this pull request in 03b5a22. |
3 participants
The
@opsdecorator provides a way to parameterize a test across a given list of ops. This would be useful for modules as well (e.g. a@modulesdecorator), but the mechanism by which this is accomplished is specific to ops. In the details, the@opsdecorator tags a test function with the metadata needed (list of ops,dtypes) and the actual tests are generated according to this metadata during the call toinstantiate_device_type_tests().This PR makes this mechanism more generic, allowing for test parameterization across arbitrary dimensions. This makes a
@modulesdecorator (or any similar type of decorator) straightforward to implement without changes to the device-specific test instantiation logic.One caveat is that, since this is implemented where the old
@opsdecorator was (withininstantiate_device_type_tests()), this only works for tests instantiated using the device-specific instantiation logic. Longer term, even device-specific test instantiation could be treated as an optional parameterization across device types, but this PR takes a low-risk approach for now. In practice, this just means that adevicekwarg is required for all test signatures used with the mechanism.The
@opsdecorator has been refactored to use the generic mechanism and works the same as before, with one difference: whenOpDTypes.noneis specified, the test signature no longer needs an unuseddtypekwarg. This is a nice bonus that demonstrates the added flexibility of a generic parameterization mechanism. The refactored form also has the bonus that all op-specific test generation logic is contained within the@opsdecorator class, improving readability.Behind the scenes, the generic mechanism is a base decorator class (
_TestParameterizer) from which@opsderives. The core functionality is in the_parameterize_test()method, which takes in a test function and returns a generator that produces parameterized tests, including names and parameter kwargs to pass to them. Using the@opsdecorator results in a set of op-specific tests from a given generic test.