Support per-parameter test decoration #79979
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[ghstack-poisoned]
🔗 Helpful links
✅ No Failures (0 Pending)As of commit d6add3c (more details on the Dr. CI page): Expand to see more💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
jbschlosser
commented
Jun 21, 2022
Comment on lines
367
to
374
| # Add the device param kwarg if the test needs device or devices. | ||
| param_kwargs = {} if param_kwargs is None else param_kwargs | ||
| test_sig_params = inspect.signature(test).parameters | ||
| if 'device' in test_sig_params or 'devices' in test_sig_params: | ||
| device_arg: str = cls.get_primary_device() | ||
| if hasattr(test, 'num_required_devices'): | ||
| device_arg = cls.get_all_devices() | ||
| _update_param_kwargs(param_kwargs, 'device', device_arg) |
There was a problem hiding this comment.
This was moved here from below so that the device kwarg is present in param_kwargs when passed to the decorator_fn().
Fixes #79161 This PR does the following: * Expands the `parametrize_fn()` signature from returning a 3-tuple of `(test, test_name, param_kwargs)` to returning a 4-tuple of `(test, test_name, param_kwargs, decorator_fn)`. Expected signature for the addition is `decorator_fn(param_kwargs) -> List[decorator]` i.e. given the full set of test params, return a list of decorators to apply. * `modules`, `ops`, and `parametrize` now fit the new signature, returning `decorator_fn`s instead of applying decorators themselves. * `instantiate_parametrized_tests()` and `instantiate_device_type_tests()` now call the returned `decorator_fn`, passing in the full set of `param_kwargs` (after composition + `device` / `dtype` additions) and applying the returned decorators. * Composing multiple `parametrize_fn`s also composes the corresponding `decorator_fn`s; the composed `decorator_fn` simply concatenates the decorator lists returned by the constituents. * Expands `DecorateInfo.is_active` to support callables: ```python DecorateInfo( unittest.expectedFailure, "TestOps", "test_python_ref_executor", device_type='cuda', active_if=lambda params: params['executor'] == 'nvfuser' ), ``` * Adds several tests to `test/test_testing.py` ensuring proper decoration using `parametrize` and `modules`. * (minor) Refactors `ModuleInfo` slightly to better match `OpInfo`; replace `should_skip()` with `get_decorators()` and merge `skips` and `decorators`. * (minor) Fixes a couple `ModuleInfo` naming oddities uncovered during testing. [ghstack-poisoned]
Fixes #79161 This PR does the following: * Expands the `parametrize_fn()` signature from returning a 3-tuple of `(test, test_name, param_kwargs)` to returning a 4-tuple of `(test, test_name, param_kwargs, decorator_fn)`. Expected signature for the addition is `decorator_fn(param_kwargs) -> List[decorator]` i.e. given the full set of test params, return a list of decorators to apply. * `modules`, `ops`, and `parametrize` now fit the new signature, returning `decorator_fn`s instead of applying decorators themselves. * `instantiate_parametrized_tests()` and `instantiate_device_type_tests()` now call the returned `decorator_fn`, passing in the full set of `param_kwargs` (after composition + `device` / `dtype` additions) and applying the returned decorators. * Composing multiple `parametrize_fn`s also composes the corresponding `decorator_fn`s; the composed `decorator_fn` simply concatenates the decorator lists returned by the constituents. * Expands `DecorateInfo.is_active` to support callables: ```python DecorateInfo( unittest.expectedFailure, "TestOps", "test_python_ref_executor", device_type='cuda', active_if=lambda params: params['executor'] == 'nvfuser' ), ``` * Adds several tests to `test/test_testing.py` ensuring proper decoration using `parametrize` and `modules`. * (minor) Refactors `ModuleInfo` slightly to better match `OpInfo`; replace `should_skip()` with `get_decorators()` and merge `skips` and `decorators`. * (minor) Fixes a couple `ModuleInfo` naming oddities uncovered during testing. [ghstack-poisoned]
Fixes #79161 This PR does the following: * Expands the `parametrize_fn()` signature from returning a 3-tuple of `(test, test_name, param_kwargs)` to returning a 4-tuple of `(test, test_name, param_kwargs, decorator_fn)`. Expected signature for the addition is `decorator_fn(param_kwargs) -> List[decorator]` i.e. given the full set of test params, return a list of decorators to apply. * `modules`, `ops`, and `parametrize` now fit the new signature, returning `decorator_fn`s instead of applying decorators themselves. * `instantiate_parametrized_tests()` and `instantiate_device_type_tests()` now call the returned `decorator_fn`, passing in the full set of `param_kwargs` (after composition + `device` / `dtype` additions) and applying the returned decorators. * Composing multiple `parametrize_fn`s also composes the corresponding `decorator_fn`s; the composed `decorator_fn` simply concatenates the decorator lists returned by the constituents. * Expands `DecorateInfo.is_active` to support callables: ```python DecorateInfo( unittest.expectedFailure, "TestOps", "test_python_ref_executor", device_type='cuda', active_if=lambda params: params['executor'] == 'nvfuser' ), ``` * Adds several tests to `test/test_testing.py` ensuring proper decoration using `parametrize` and `modules`. * (minor) Refactors `ModuleInfo` slightly to better match `OpInfo`; replace `should_skip()` with `get_decorators()` and merge `skips` and `decorators`. * (minor) Fixes a couple `ModuleInfo` naming oddities uncovered during testing. [ghstack-poisoned]
Fixes #79161 This PR does the following: * Expands the `parametrize_fn()` signature from returning a 3-tuple of `(test, test_name, param_kwargs)` to returning a 4-tuple of `(test, test_name, param_kwargs, decorator_fn)`. Expected signature for the addition is `decorator_fn(param_kwargs) -> List[decorator]` i.e. given the full set of test params, return a list of decorators to apply. * `modules`, `ops`, and `parametrize` now fit the new signature, returning `decorator_fn`s instead of applying decorators themselves. * `instantiate_parametrized_tests()` and `instantiate_device_type_tests()` now call the returned `decorator_fn`, passing in the full set of `param_kwargs` (after composition + `device` / `dtype` additions) and applying the returned decorators. * Composing multiple `parametrize_fn`s also composes the corresponding `decorator_fn`s; the composed `decorator_fn` simply concatenates the decorator lists returned by the constituents. * Expands `DecorateInfo.is_active` to support callables: ```python DecorateInfo( unittest.expectedFailure, "TestOps", "test_python_ref_executor", device_type='cuda', active_if=lambda params: params['executor'] == 'nvfuser' ), ``` * Adds several tests to `test/test_testing.py` ensuring proper decoration using `parametrize` and `modules`. * (minor) Refactors `ModuleInfo` slightly to better match `OpInfo`; replace `should_skip()` with `get_decorators()` and merge `skips` and `decorators`. * (minor) Fixes a couple `ModuleInfo` naming oddities uncovered during testing. [ghstack-poisoned]
Fixes #79161 This PR does the following: * Expands the `parametrize_fn()` signature from returning a 3-tuple of `(test, test_name, param_kwargs)` to returning a 4-tuple of `(test, test_name, param_kwargs, decorator_fn)`. Expected signature for the addition is `decorator_fn(param_kwargs) -> List[decorator]` i.e. given the full set of test params, return a list of decorators to apply. * `modules`, `ops`, and `parametrize` now fit the new signature, returning `decorator_fn`s instead of applying decorators themselves. * `instantiate_parametrized_tests()` and `instantiate_device_type_tests()` now call the returned `decorator_fn`, passing in the full set of `param_kwargs` (after composition + `device` / `dtype` additions) and applying the returned decorators. * Composing multiple `parametrize_fn`s also composes the corresponding `decorator_fn`s; the composed `decorator_fn` simply concatenates the decorator lists returned by the constituents. * Expands `DecorateInfo.is_active` to support callables: ```python DecorateInfo( unittest.expectedFailure, "TestOps", "test_python_ref_executor", device_type='cuda', active_if=lambda params: params['executor'] == 'nvfuser' ), ``` * Adds several tests to `test/test_testing.py` ensuring proper decoration using `parametrize` and `modules`. * (minor) Refactors `ModuleInfo` slightly to better match `OpInfo`; replace `should_skip()` with `get_decorators()` and merge `skips` and `decorators`. * (minor) Fixes a couple `ModuleInfo` naming oddities uncovered during testing. [ghstack-poisoned]
Fixes #79161 This PR does the following: * Expands the `parametrize_fn()` signature from returning a 3-tuple of `(test, test_name, param_kwargs)` to returning a 4-tuple of `(test, test_name, param_kwargs, decorator_fn)`. Expected signature for the addition is `decorator_fn(param_kwargs) -> List[decorator]` i.e. given the full set of test params, return a list of decorators to apply. * `modules`, `ops`, and `parametrize` now fit the new signature, returning `decorator_fn`s instead of applying decorators themselves. * `instantiate_parametrized_tests()` and `instantiate_device_type_tests()` now call the returned `decorator_fn`, passing in the full set of `param_kwargs` (after composition + `device` / `dtype` additions) and applying the returned decorators. * Composing multiple `parametrize_fn`s also composes the corresponding `decorator_fn`s; the composed `decorator_fn` simply concatenates the decorator lists returned by the constituents. * Expands `DecorateInfo.is_active` to support callables: ```python DecorateInfo( unittest.expectedFailure, "TestOps", "test_python_ref_executor", device_type='cuda', active_if=lambda params: params['executor'] == 'nvfuser' ), ``` * Adds several tests to `test/test_testing.py` ensuring proper decoration using `parametrize` and `modules`. * (minor) Refactors `ModuleInfo` slightly to better match `OpInfo`; replace `should_skip()` with `get_decorators()` and merge `skips` and `decorators`. * (minor) Fixes a couple `ModuleInfo` naming oddities uncovered during testing. [ghstack-poisoned]
jbschlosser
commented
Jun 29, 2022
Comment on lines
+323
to
+332
| @classmethod | ||
| def _init_and_get_primary_device(cls): | ||
| try: | ||
| return cls.get_primary_device() | ||
| except Exception: | ||
| # For CUDATestBase, XLATestBase, and possibly others, the primary device won't be available | ||
| # until setUpClass() sets it. Call that manually here if needed. | ||
| if hasattr(cls, 'setUpClass'): | ||
| cls.setUpClass() | ||
| return cls.get_primary_device() |
There was a problem hiding this comment.
This is a bit messy. The device specific test class for CUDA, XLA, and maybe others (?) define setupUpClass() method that populates the primary device. In the past, this has been run by unittest just before the first test invocation. Unfortunately, that's too late if we want the per-param decoration mechanism to have access to it during test generation time.
The hacky approach here calls setUpClass() if trying to get the primary device fails. Open to better suggestions! Note that I have been operating under the assumption that there may be other device-specific test classes out there we don't know about or can't easily change. If we aren't restricted by this, other options involving changing CUDATestBase / XLATestBase open up.
There was a problem hiding this comment.
I also can't immediately think of a better way to do this; I found the comment very helpful
mruberry
reviewed
Jul 7, 2022
| @@ -1494,6 +1500,48 @@ def test_two_things_custom_names(self, x, y): | |||
| test_names = _get_test_names_for_test_class(TestParametrized) | |||
| self.assertEqual(expected_test_names, test_names) | |||
|
|
|||
| def test_apply_param_specific_decorators(self): | |||
mruberry
reviewed
Jul 7, 2022
| @@ -1506,7 +1554,7 @@ def test_modules(self, module_info): | |||
| instantiate_parametrized_tests(TestParametrized) | |||
|
|
|||
| def test_ops_decorator_misuse_error(self): | |||
| # Test that @modules errors out when used with instantiate_parametrized_tests(). | |||
| # Test that @ops errors out when used with instantiate_parametrized_tests(). | |||
mruberry
reviewed
Jul 7, 2022
| @@ -1685,6 +1733,81 @@ def test_op_parametrized(self, device, dtype, op, flag): | |||
| test_names = _get_test_names_for_test_class(device_cls) | |||
| self.assertEqual(sorted(expected_test_names), sorted(test_names)) | |||
|
|
|||
| def test_modules_composition_names(self, device): | |||
| device = self.device_type | |||
There was a problem hiding this comment.
Comment here, too, please
mruberry
approved these changes
Jul 7, 2022
|
Is there some documentation this should update? Maybe a comment like the one describing the OpInfos or the device-generic test framework? |
Fixes #79161 This PR does the following: * Expands the `parametrize_fn()` signature from returning a 3-tuple of `(test, test_name, param_kwargs)` to returning a 4-tuple of `(test, test_name, param_kwargs, decorator_fn)`. Expected signature for the addition is `decorator_fn(param_kwargs) -> List[decorator]` i.e. given the full set of test params, return a list of decorators to apply. * `modules`, `ops`, and `parametrize` now fit the new signature, returning `decorator_fn`s instead of applying decorators themselves. * `instantiate_parametrized_tests()` and `instantiate_device_type_tests()` now call the returned `decorator_fn`, passing in the full set of `param_kwargs` (after composition + `device` / `dtype` additions) and applying the returned decorators. * Composing multiple `parametrize_fn`s also composes the corresponding `decorator_fn`s; the composed `decorator_fn` simply concatenates the decorator lists returned by the constituents. * Expands `DecorateInfo.is_active` to support callables: ```python DecorateInfo( unittest.expectedFailure, "TestOps", "test_python_ref_executor", device_type='cuda', active_if=lambda params: params['executor'] == 'nvfuser' ), ``` * Adds several tests to `test/test_testing.py` ensuring proper decoration using `parametrize` and `modules`. * (minor) Refactors `ModuleInfo` slightly to better match `OpInfo`; replace `should_skip()` with `get_decorators()` and merge `skips` and `decorators`. * (minor) Fixes a couple `ModuleInfo` naming oddities uncovered during testing. [ghstack-poisoned]
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
|
/easycla As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details. This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign. |
|
|
jbschlosser
added a commit
that referenced
this pull request
Jan 4, 2023
Continuation of #79979. Fixes #79161 This PR does the following: * Expands the `parametrize_fn()` signature from returning a 3-tuple of `(test, test_name, param_kwargs)` to returning a 4-tuple of `(test, test_name, param_kwargs, decorator_fn)`. Expected signature for the addition is `decorator_fn(param_kwargs) -> List[decorator]` i.e. given the full set of test params, return a list of decorators to apply. * `modules`, `ops`, and `parametrize` now fit the new signature, returning `decorator_fn`s instead of applying decorators themselves. * `instantiate_parametrized_tests()` and `instantiate_device_type_tests()` now call the returned `decorator_fn`, passing in the full set of `param_kwargs` (after composition + `device` / `dtype` additions) and applying the returned decorators. * Composing multiple `parametrize_fn`s also composes the corresponding `decorator_fn`s; the composed `decorator_fn` simply concatenates the decorator lists returned by the constituents. * Expands `DecorateInfo.is_active` to support callables: ```python DecorateInfo( unittest.expectedFailure, "TestOps", "test_python_ref_executor", device_type='cuda', active_if=lambda params: params['executor'] == 'nvfuser' ), ``` * Adds several tests to `test/test_testing.py` ensuring proper decoration using `parametrize`, `modules`, and `ops`. * (minor) Fixes a couple `ModuleInfo` naming oddities uncovered during testing. [ghstack-poisoned]
jbschlosser
added a commit
that referenced
this pull request
Jan 4, 2023
Continuation of #79979. Fixes #79161 This PR does the following: * Expands the `parametrize_fn()` signature from returning a 3-tuple of `(test, test_name, param_kwargs)` to returning a 4-tuple of `(test, test_name, param_kwargs, decorator_fn)`. Expected signature for the addition is `decorator_fn(param_kwargs) -> List[decorator]` i.e. given the full set of test params, return a list of decorators to apply. * `modules`, `ops`, and `parametrize` now fit the new signature, returning `decorator_fn`s instead of applying decorators themselves. * `instantiate_parametrized_tests()` and `instantiate_device_type_tests()` now call the returned `decorator_fn`, passing in the full set of `param_kwargs` (after composition + `device` / `dtype` additions) and applying the returned decorators. * Composing multiple `parametrize_fn`s also composes the corresponding `decorator_fn`s; the composed `decorator_fn` simply concatenates the decorator lists returned by the constituents. * Expands `DecorateInfo.is_active` to support callables: ```python DecorateInfo( unittest.expectedFailure, "TestOps", "test_python_ref_executor", device_type='cuda', active_if=lambda params: params['executor'] == 'nvfuser' ), ``` * Adds several tests to `test/test_testing.py` ensuring proper decoration using `parametrize`, `modules`, and `ops`. * (minor) Fixes a couple `ModuleInfo` naming oddities uncovered during testing. [ghstack-poisoned]
pytorchmergebot
pushed a commit
that referenced
this pull request
Jan 4, 2023
Continuation of #79979. Fixes #79161 This PR does the following: * Expands the `parametrize_fn()` signature from returning a 3-tuple of `(test, test_name, param_kwargs)` to returning a 4-tuple of `(test, test_name, param_kwargs, decorator_fn)`. Expected signature for the addition is `decorator_fn(param_kwargs) -> List[decorator]` i.e. given the full set of test params, return a list of decorators to apply. * `modules`, `ops`, and `parametrize` now fit the new signature, returning `decorator_fn`s instead of applying decorators themselves. * `instantiate_parametrized_tests()` and `instantiate_device_type_tests()` now call the returned `decorator_fn`, passing in the full set of `param_kwargs` (after composition + `device` / `dtype` additions) and applying the returned decorators. * Composing multiple `parametrize_fn`s also composes the corresponding `decorator_fn`s; the composed `decorator_fn` simply concatenates the decorator lists returned by the constituents. * Expands `DecorateInfo.is_active` to support callables: ```python DecorateInfo( unittest.expectedFailure, "TestOps", "test_python_ref_executor", device_type='cuda', active_if=lambda params: params['executor'] == 'nvfuser' ), ``` * Adds several tests to `test/test_testing.py` ensuring proper decoration using `@parametrize`, `@modules`, and `@ops`. * (minor) Fixes a couple `ModuleInfo` naming oddities uncovered during testing. Pull Request resolved: #91658 Approved by: https://github.com/malfet
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack:
Fixes #79161
This PR does the following:
parametrize_fn()signature from returning a 3-tuple of(test, test_name, param_kwargs)to returning a 4-tuple of(test, test_name, param_kwargs, decorator_fn). Expected signature for the addition isdecorator_fn(param_kwargs) -> List[decorator]i.e. given the full set of test params, return a list of decorators to apply.@modules,@ops, and@parametrizenow fit the new signature, returningdecorator_fns instead of applying decorators themselves.instantiate_parametrized_tests()andinstantiate_device_type_tests()now call the returneddecorator_fn, passing in the full set ofparam_kwargs(after composition +device/dtypeadditions) and applying the returned decorators.parametrize_fns also composes the correspondingdecorator_fns; the composeddecorator_fnsimply concatenates the decorator lists returned by the constituents.DecorateInfo.is_activeto support callables:test/test_testing.pyensuring proper decoration using@parametrizeand@modules.ModuleInfoslightly to better matchOpInfo; replaceshould_skip()withget_decorators()and mergeskipsanddecorators.ModuleInfonaming oddities uncovered during testing.