Generic test parametrization functionality

[jbschlosser]

This PR plays around with implementation & usage of a @parametrize decorator for test parametrization similar to @pytest.mark.parametrize, based on previous work introducing a _TestParametrizer class. It works with the internal DeviceTest hierarchy & composes with @dtype, @skip*, and other decorators. Basic usage is demonstrated in test/test_blah.py:

import unittest
from itertools import product
from torch.testing._internal.common_device_type import (
    instantiate_device_type_tests, deviceCountAtLeast, ops)
from torch.testing._internal.common_methods_invocations import op_db
from torch.testing._internal.common_utils import (
    TestCase, run_tests, parametrize, instantiate_parametrized_tests, subtest)


class TestBlah(TestCase):
    @parametrize("x", range(5))
    def test_default_names(self, x):
        print('Passed in:', x)

    # Use default names but add an expected failure.
    @parametrize("x", [subtest(0, decorators=[unittest.expectedFailure]),
                       *range(1, 5)])
    def test_default_names_expected_failure(self, x):
        if x == 0:
            raise RuntimeError('Boom')
        print('Passed in:', x)

    @parametrize("bias", [False, True], name_fn=lambda b: 'bias' if b else 'no_bias')
    def test_custom_names(self, bias):
        print('Passed in:', bias)

    @parametrize("bias", [subtest(True, name='bias'),
                          subtest(False, name='no_bias')])
    def test_custom_names_alternate(self, bias):
        print('Passed in:', bias)

    @parametrize("x,y", [(1, 2), (1, 3), (1, 4)])
    def test_two_things_default_names(self, x, y):
        print('Passed in:', x, y)

    @parametrize("x", [1, 2, 3])
    @parametrize("y", [4, 5, 6])
    def test_two_things_composition(self, x, y):
        print('Passed in:', x, y)

    @parametrize("x", [subtest(0, decorators=[unittest.expectedFailure]),
                       *range(1, 3)])
    @parametrize("y", [4, 5, subtest(6, decorators=[unittest.expectedFailure])])
    def test_two_things_composition_expected_failure(self, x, y):
        if x == 0 or y == 6:
            raise RuntimeError('Boom')
        print('Passed in:', x, y)

    @parametrize("x", [1, 2])
    @parametrize("y", [3, 4])
    @parametrize("z", [5, 6])
    def test_three_things_composition(self, x, y, z):
        print('Passed in:', x, y, z)

    @parametrize("x", [1, 2], name_fn=str)
    @parametrize("y", [3, 4], name_fn=str)
    @parametrize("z", [5, 6], name_fn=str)
    def test_three_things_composition_custom_names(self, x, y, z):
        print('Passed in:', x, y, z)

    @parametrize("x,y", product(range(2), range(3)))
    def test_two_things_product(self, x, y):
        print('Passed in:', x, y)

    @parametrize("x,y", [subtest((1, 2), name='double'),
                         subtest((1, 3), name='triple'),
                         subtest((1, 4), name='quadruple')])
    def test_two_things_custom_names(self, x, y):
        print('Passed in:', x, y)

    @parametrize("x,y", [(1, 2), (1, 3), (1, 4)], name_fn=lambda x, y: '{}_{}'.format(x, y))
    def test_two_things_custom_names_alternate(self, x, y):
        print('Passed in:', x, y)


class TestDeviceBlah(TestCase):
    @parametrize("x", range(10))
    def test_default_names(self, device, x):
        print('Passed in:', device, x)

    @parametrize("x,y", [(1, 2), (3, 4), (5, 6)])
    def test_two_things(self, device, x, y):
        print('Passed in:', device, x, y)

    @deviceCountAtLeast(1)
    def test_multiple_devices(self, devices):
        print('Passed in:', devices)

    @ops(op_db)
    @parametrize("flag", [False, True], lambda f: 'flag_enabled' if f else 'flag_disabled')
    def test_op_parametrized(self, device, dtype, op, flag):
        print('Passed in:', device, dtype, op, flag)


instantiate_parametrized_tests(TestBlah)
instantiate_device_type_tests(TestDeviceBlah, globals())


if __name__ == '__main__':
    run_tests()

Generated tests:

TestBlah.test_custom_names_alternate_bias
TestBlah.test_custom_names_alternate_no_bias
TestBlah.test_custom_names_bias
TestBlah.test_custom_names_no_bias
TestBlah.test_default_names_expected_failure_x_0
TestBlah.test_default_names_expected_failure_x_1
TestBlah.test_default_names_expected_failure_x_2
TestBlah.test_default_names_expected_failure_x_3
TestBlah.test_default_names_expected_failure_x_4
TestBlah.test_default_names_x_0
TestBlah.test_default_names_x_1
TestBlah.test_default_names_x_2
TestBlah.test_default_names_x_3
TestBlah.test_default_names_x_4
TestBlah.test_three_things_composition_custom_names_1_3_5
TestBlah.test_three_things_composition_custom_names_1_3_6
TestBlah.test_three_things_composition_custom_names_1_4_5
TestBlah.test_three_things_composition_custom_names_1_4_6
TestBlah.test_three_things_composition_custom_names_2_3_5
TestBlah.test_three_things_composition_custom_names_2_3_6
TestBlah.test_three_things_composition_custom_names_2_4_5
TestBlah.test_three_things_composition_custom_names_2_4_6
TestBlah.test_three_things_composition_x_1_y_3_z_5
TestBlah.test_three_things_composition_x_1_y_3_z_6
TestBlah.test_three_things_composition_x_1_y_4_z_5
TestBlah.test_three_things_composition_x_1_y_4_z_6
TestBlah.test_three_things_composition_x_2_y_3_z_5
TestBlah.test_three_things_composition_x_2_y_3_z_6
TestBlah.test_three_things_composition_x_2_y_4_z_5
TestBlah.test_three_things_composition_x_2_y_4_z_6
TestBlah.test_two_things_composition_expected_failure_x_0_y_4
TestBlah.test_two_things_composition_expected_failure_x_0_y_5
TestBlah.test_two_things_composition_expected_failure_x_0_y_6
TestBlah.test_two_things_composition_expected_failure_x_1_y_4
TestBlah.test_two_things_composition_expected_failure_x_1_y_5
TestBlah.test_two_things_composition_expected_failure_x_1_y_6
TestBlah.test_two_things_composition_expected_failure_x_2_y_4
TestBlah.test_two_things_composition_expected_failure_x_2_y_5
TestBlah.test_two_things_composition_expected_failure_x_2_y_6
TestBlah.test_two_things_composition_x_1_y_4
TestBlah.test_two_things_composition_x_1_y_5
TestBlah.test_two_things_composition_x_1_y_6
TestBlah.test_two_things_composition_x_2_y_4
TestBlah.test_two_things_composition_x_2_y_5
TestBlah.test_two_things_composition_x_2_y_6
TestBlah.test_two_things_composition_x_3_y_4
TestBlah.test_two_things_composition_x_3_y_5
TestBlah.test_two_things_composition_x_3_y_6
TestBlah.test_two_things_custom_names_alternate_1_2
TestBlah.test_two_things_custom_names_alternate_1_3
TestBlah.test_two_things_custom_names_alternate_1_4
TestBlah.test_two_things_custom_names_double
TestBlah.test_two_things_custom_names_quadruple
TestBlah.test_two_things_custom_names_triple
TestBlah.test_two_things_default_names_x_1_y_2
TestBlah.test_two_things_default_names_x_1_y_3
TestBlah.test_two_things_default_names_x_1_y_4
TestBlah.test_two_things_product_x_0_y_0
TestBlah.test_two_things_product_x_0_y_1
TestBlah.test_two_things_product_x_0_y_2
TestBlah.test_two_things_product_x_1_y_0
TestBlah.test_two_things_product_x_1_y_1
TestBlah.test_two_things_product_x_1_y_2
TestDeviceBlahCPU.test_default_names_x_0_cpu
TestDeviceBlahCPU.test_default_names_x_1_cpu
TestDeviceBlahCPU.test_default_names_x_2_cpu
TestDeviceBlahCPU.test_default_names_x_3_cpu
TestDeviceBlahCPU.test_default_names_x_4_cpu
TestDeviceBlahCPU.test_default_names_x_5_cpu
TestDeviceBlahCPU.test_default_names_x_6_cpu
TestDeviceBlahCPU.test_default_names_x_7_cpu
TestDeviceBlahCPU.test_default_names_x_8_cpu
TestDeviceBlahCPU.test_default_names_x_9_cpu
TestDeviceBlahCPU.test_multiple_devices_cpu
TestDeviceBlahCPU.test_op_parametrized_<opname>_<variant>_cpu_uint8_flag_enabled_cpu
TestDeviceBlahCPU.test_two_things_x_1_y_2_cpu
TestDeviceBlahCPU.test_two_things_x_3_y_4_cpu
TestDeviceBlahCPU.test_two_things_x_5_y_6_cpu
TestDeviceBlahMETA.test_default_names_x_0_meta
TestDeviceBlahMETA.test_default_names_x_1_meta
TestDeviceBlahMETA.test_default_names_x_2_meta
TestDeviceBlahMETA.test_default_names_x_3_meta
TestDeviceBlahMETA.test_default_names_x_4_meta
TestDeviceBlahMETA.test_default_names_x_5_meta
TestDeviceBlahMETA.test_default_names_x_6_meta
TestDeviceBlahMETA.test_default_names_x_7_meta
TestDeviceBlahMETA.test_default_names_x_8_meta
TestDeviceBlahMETA.test_default_names_x_9_meta
TestDeviceBlahMETA.test_multiple_devices_meta
TestDeviceBlahMETA.test_op_parametrized_<opname>_<variant>_meta_uint8_flag_enabled_meta
TestDeviceBlahMETA.test_two_things_x_1_y_2_meta
TestDeviceBlahMETA.test_two_things_x_3_y_4_meta
TestDeviceBlahMETA.test_two_things_x_5_y_6_meta

Caveats:

• @parametrize decorators cannot be "stacked" yet; each one overwrites the previous. This will change to either:
  • Allow stacking of multiple decorators
  • Error out with a nice error message if multiple decorators are specified

The PR introduces instantiate_parametrized_tests() in addition to instantiate_device_type_tests(). The former should be used for non-device-specific tests, and the latter should be used for device-specific tests, as usual. Both of these support the @parametrize decorator. Only the latter supports the @ops decorator (no change here- this was already the case).

[facebook-github-bot]

🔗 Helpful links

• 🧪  See artifacts and rendered test results at hud.pytorch.org/pr/60753
• 📄  Preview docs built from this PR
• 📄  Preview C++ docs built from this PR
• ↩️  [fb-only] Re-run with SSH instructions
• 🔧  Opt-in to CIFlow to control what jobs run on your PRs

💊 CI failures summary and remediations

As of commit 1358d20 (more details on the Dr. CI page):

---

• 1/1 failures introduced in this PR

---

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

linux-bionic-py3.6-clang9 / test (default, 2, 2, linux.2xlarge) (1/1)

Step: "Test PyTorch" (full log | diagnosis details | 🔁 rerun)

2021-09-14T19:40:43.1704705Z RuntimeError: test_jit failed! Received signal: SIGSEGV

2021-09-14T19:40:42.5871884Z   test_pretty_printer (__main__.TestJit) ... ok (0.026s)
2021-09-14T19:40:42.5878553Z   test_print_classes_module (__main__.TestJit) ... ok (0.001s)
2021-09-14T19:40:42.5885837Z   test_print_op_module (__main__.TestJit) ... ok (0.001s)
2021-09-14T19:40:42.5893371Z   test_print_torch_ops_modules (__main__.TestJit) ... ok (0.001s)
2021-09-14T19:40:42.6002619Z   test_profiler (__main__.TestJit) ... ok (0.011s)
2021-09-14T19:40:43.1699920Z   test_python_bindings (__main__.TestJit) ... Traceback (most recent call last):
2021-09-14T19:40:43.1700539Z   File "test/run_test.py", line 1027, in <module>
2021-09-14T19:40:43.1701751Z     main()
2021-09-14T19:40:43.1702146Z   File "test/run_test.py", line 1005, in main
2021-09-14T19:40:43.1704169Z     raise RuntimeError(err_message)
2021-09-14T19:40:43.1704705Z RuntimeError: test_jit failed! Received signal: SIGSEGV
2021-09-14T19:40:43.4086084Z 
2021-09-14T19:40:43.4086817Z real	13m39.839s
2021-09-14T19:40:43.4087114Z user	36m1.341s
2021-09-14T19:40:43.4087382Z sys	3m2.015s
2021-09-14T19:40:43.4087661Z + cleanup
2021-09-14T19:40:43.4087950Z + retcode=1
2021-09-14T19:40:43.4088219Z + set +x
2021-09-14T19:40:43.4088570Z =================== sccache compilation log ===================
2021-09-14T19:40:43.4290020Z =========== If your build fails, please take a look at the log above for possible reasons ===========
2021-09-14T19:40:43.4312462Z Compile requests                     42

---

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.

Click here to manually regenerate this comment.

[mruberry]

Is there an existing test that could be used as an example instead of these samples?

[zou3519]

pytorch/test/test_autograd.py

Line 4301 in c74c0c5

def test_gradcheck_forward_ad(self):

is parameterized over fast_mode in [True, False]

pytorch/test/test_autograd.py

Lines 3915 to 3923 in c74c0c5

	def test_gradcheck_single_input(self):
	def check(fast_mode):
	def f(inp):
	return inp.mul(5)
	gradcheck(f, torch.rand(10, dtype=torch.float64, requires_grad=True), fast_mode=fast_mode)
	gradgradcheck(f, torch.rand(10, dtype=torch.float64, requires_grad=True), fast_mode=fast_mode)
	check(fast_mode=True)
	check(fast_mode=False)

is also parametrized in the same way (a lot of the other gradcheck tests are this way too)

[zou3519]

To add a data point... I like having the samples in test_blah as test cases because if something starts failing then it is clearer the failure is due to the parameterize functionality and not e.g. a bug in PyTorch operator code

[mruberry]

We can add tests to test_testing.py (there are some examples) if we want to preserve testing the decorator without a specific test

[jbschlosser]

Yes, I definitely agree on adding tests to test_testing.py for these decorators. To @zou3519's point, being able to distinguish parametrization failures vs. real test failures is crucial.

Just dumped them in test_blah.py for the sake of demonstration, but they should definitely be moved to the proper permanent location, and test_testing.py seems right :)

[mruberry]

These tests should accept a device argument and use it. The argument is not a device type but a string representing an actual device. Like "cuda:0" or "cuda:1", but these tests are ignoring the test device in favor of "cuda:0"

[jbschlosser]

This has been fixed in the previous parametrization PR- once I rebase the change will show up here too

[mruberry]

For language consistency the decorated function is the test "template" which is instantiated into the actual tests

[mruberry]

Follow-up on this, this is really the base class to inherit from if writing such a decorator, right? A pointer to an example of a class that inherits from this in the docs here would be neat, too

[mruberry]

Does pytest allow for naming of the variants?

[zou3519]

agree that variant naming is nice to have (and I'd consider adding it even if pytest doesn't have it)

[jbschlosser]

So my understanding is that pytest's parametrize only works with pytest tests and not unittest (see here).

What this means is that pytest has an internal way of doing variant naming. I don't believe it adds e.g. a test_func_variant_a() function to the test class for every variant, in contrast to unittest's modus operandi. So I think we'll have to add some custom mechanism for doing the naming to stay compatible with unittest.

[mruberry]

If people name the variants I think it's straightforward to generate a test for each of them. If people don't name the variants but the variants are a sequence with a fixed length (like a list or tuple) then we can probably name them _0 through _n. Where naming gets tricky is if we have a variable-length variant generator (like sample inputs), because our test template instantiation happens at "compile time" and we probably don't want to enumerate arbitrary generators just to discover how many elements they have. If people are really keen on using generators maybe we can have them pass the length as a compile-time constant?

[jbschlosser]

Added some updates that allow for variant naming. There are two mechanisms for doing so, and whichever is most convenient can be used:

• Specify name_mappings (open to other names), which is a list of either dicts of callables that map a parameter value to its string representation. The test suffix is formed by joining parameter string representations with underscores.
• Specify suffix_fn, which is a callable that takes in the parameter values and returns a test suffix. This can be used when full control over the suffix is needed and it shouldn't just be an underscore-separated string.

If neither are specified, the default test suffix consists of the lowercase string representation for each parameter value separated by underscores.

Check out the updated PR description for examples!

[mruberry]

How important is each variant being instantiated in its own test, @zou3519?

[zou3519]

It's pretty important but I wouldn't block this PR on this. FWIW pytest also generates a test case per variant.

The main argument for each variant getting its own test is because it makes it easier to tell exactly what variant a test case failed for. For example, if I have some autograd tests that are parametrized over being for "reverse-mode" or "forward-mode" AD (this happens in some tests in test/test_autograd.py) if a test fails I want to be able to know if reverse-mode is broken, if forward-mode is broken, or if both are broken.

[jbschlosser]

Definitely need variants as individual tests, I agree! See my comment above for how pytest does this, from what I can tell. Given that, there's a UX discussion to be had on the right way to allow devs to name their variants.

[mruberry]

This is really cool; I made a few comments inline.

Throwing an error when multiple decorators are used sounds good because pytest does allow multiple parametrize decorators to be used and takes their cross-product, so that's a scenario that users might reasonable expect to work the same way

[jbschlosser]

Throwing an error when multiple decorators are used sounds good because pytest does allow multiple parametrize decorators to be used and takes their cross-product, so that's a scenario that users might reasonable expect to work the same way

Definitely agree, don't want this confusion. One alternative is that we also do the cross-product here? I have to fully think through how composing @ops with @parametrize would work though. I think there's a way of restricting composability to only multiple @parametrizes with some extra function decoration..

@mruberry How useful do you think being able to compose @ops with @parametrize would be? Any op tests that would benefit from an additional parameter?

[jbschlosser]

I pushed an update that does test per variant, but the naming is currently pretty bad (it uses concatenated lower-case string representations of the param values):

class TestFoo(unittest.TestCase):
  @parametrize("x,y", [(1, 2), (3, 4)])
  def test_bar():
    ...

results in these test names:

TestFoo.test_bar_1_3
TestFoo.test_bar_2_4

[mruberry]

Throwing an error when multiple decorators are used sounds good because pytest does allow multiple parametrize decorators to be used and takes their cross-product, so that's a scenario that users might reasonable expect to work the same way

Definitely agree, don't want this confusion. One alternative is that we also do the cross-product here? I have to fully think through how composing @ops with @parametrize would work though. I think there's a way of restricting composability to only multiple @parametrizes with some extra function decoration..

Sure! We could also cut this PR's features for code velocity and create a follow-up issue with tracking tasks.

@mruberry How useful do you think being able to compose @ops with @parametrize would be? Any op tests that would benefit from an additional parameter?

I think there are cases where this could be useful but wouldn't require this as a v0 feature.

[heitorschueroff]

How about having arg_values=None to indicate a boolean parameter? In this case, two versions of the test would be created, and instead of appending the value to the test name, append arg_str only for the True case. This could be great for generating versions based on some boolean flag parameter.

[jbschlosser]

Good idea - I think it might be cool to have some magic for the very common case of boolean flags.

[heitorschueroff]

Would this allow for multiple @parametrize to test the product of some parameters?

[jbschlosser]

Currently, no. I'm looking into two types of composability for @parametrize:

1. Product-wise composability as you mentioned (each @parametrize instance specifies a subset of args and tests are generated from the product)
2. Additive composability (each @parametrize instance specifies all args, adding to the tests that are run)

Technically, it might be possible to have both, but I'm leaning towards only providing the latter. I think if a true product is desired, it can be generated inline manually. Do you have a strong opinion either way?

[jbschlosser]

Update here: I ended up adding product-wise composability since several people I talked to expected it.

[zou3519]

Suggestion: If the case list has an additional element (3 instead of 2), then we interpret the first element as the "test variant name" (or the suffix).

@parametrize("x,y", [('foo', 1, 2), ('bar', 1, 3), ('baz', 1, 4)])

Otherwise, the user is required to specify a suffix_fn or name_mapping_fn:

@parametrize("x,y", [(1, 2), (1, 3), (1, 4)], suffix_fn=...)

Alternatively, if the case list is a Dict[str, Tuple] instead of List[Tuple], then we interpret the key as the "test variant name":

@parametrize("x,y", {'foo': (1, 2), 'bar': (1, 3), 'baz': (1, 4)})

[jbschlosser]

Suggestion: If the case list has an additional element (3 instead of 2), then we interpret the first element as the "test variant name" (or the suffix).

@parametrize("x,y", [('foo', 1, 2), ('bar', 1, 3), ('baz', 1, 4)])

While this does work when there are multiple parameters, there's some ambiguity for the single parameter case:

# Should we use custom names OR parametrize where each x is a (str, int)? Either could be valid.
@parametrize("x", [('foo', 1), ('bar', 2), ('baz', 3)])

Alternatively, if the case list is a Dict[str, Tuple] instead of List[Tuple], then we interpret the key as the "test variant name":

@parametrize("x,y", {'foo': (1, 2), 'bar': (1, 3), 'baz': (1, 4)})

I like this better, but what if you want to mark 'foo' as an expected failure, for example? You'd need to rewrite the whole thing to use the subtest structure. Maybe that's okay?

[zou3519]

I like this better, but what if you want to mark 'foo' as an expected failure, for example? You'd need to rewrite the whole thing to use the subtest structure. Maybe that's okay?

Good point. Maybe we should just use the subtest structure. The only downside is that it could be verbose but it's gives us a good mix of things we want (as you noted, (1) it makes it easier to add expected failures and (2) it satisfies our constraint of keeping the variant name with the case)

[jbschlosser]

Pushed an update converging on the "subtest" terminology and removing most of the ways to specify subtest names.

[jbschlosser]

Good feedback from @albanD posted here for posterity: make sure it's easy to ctrl-f for test names within the code. Generated test names make this harder; it could help to make very clear what is the searchable base name part and what is the variant name part.

[mruberry]

@mruberry How useful do you think being able to compose @ops with @parametrize would be? Any op tests that would benefit from an additional parameter?

I think so. In fact, I think @heitorschueroff has a PR where he'd like to do exactly this

[mruberry]

Naming is hard

[mruberry]

This looks really good, @jbschlosser - I just added a few inline comments for your review

[codecov]

Codecov Report

Merging #60753 (1358d20) into master (3d976d9) will decrease coverage by 0.26%.
The diff coverage is 86.56%.

@@            Coverage Diff             @@
##           master   #60753      +/-   ##
==========================================
- Coverage   66.69%   66.42%   -0.27%     
==========================================
  Files         718      724       +6     
  Lines       92693    93271     +578     
==========================================
+ Hits        61817    61958     +141     
- Misses      30876    31313     +437     

[facebook-github-bot]

@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[mruberry]

Hey @jbschlosser! This is super cool (I encourage you to do a post on it and update some docs to let people know they have a parametrize decorator). I left just a few super minor comments about possible opportunities to improve the docs, but nothing blocking.

[facebook-github-bot]

@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@jbschlosser merged this pull request in b7ec7d7.