Initial ModuleInfo implementation #61935

jbschlosser · 2021-07-20T21:25:23Z

This PR contains the initial version of ModuleInfo for use in testing modules. The design philosophy taken here is to start small and simple and build out / refactor as needed when more test coverage or ModuleInfo entries are added. As such, it's not intended for general usage yet. The PR contains the following:

(new file) torch/testing/_internal/common_modules.py
- ModuleInfo definition - metadata for each module to use in testing
- module_db - the actual ModuleInfo database; currently contains entries for two modules
- ModuleInput - analogous to SampleInput from OpInfo; contains FunctionInputs for both constructor and forward pass inputs
  - Constructor and forward pass inputs are tied together within a ModuleInput because they are likely correlated
- FunctionInput - just contains args and kwargs to pass to a function (is there a nicer way to do this?)
- @modules decorator - analogous to @ops; specifies a set of modules to run a test over
- Some constants used to keep track of all modules under torch.nn:
  - MODULE_NAMESPACES - list of all namespaces containing modules
  - MODULE_CLASSES - list of all module class objects
  - MODULE_CLASS_NAMES - dict from module class object to nice name (e.g. torch.nn.Linear -> "nn.Linear")
(new file) test/test_modules.py
- Uses the above to define tests over modules
- Currently, there is one test for demonstration, test_forward, which instantiates a module, runs its forward pass, and compares it to a reference, if one is defined

facebook-github-bot · 2021-07-20T21:25:29Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/61935
📄 Preview docs built from this PR

💊 CI failures summary and remediations

As of commit 3c4ecc8 (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:

pytorch_linux_bionic_cuda10_2_cudnn7_py3_9_gcc7_test2 (1/1)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Jul 23 19:13:56 AssertionError: False is not tr...780061185359955), which occurred at index (4, 14).

Jul 23 19:13:56   File "/var/lib/jenkins/workspace/test/test_foreach.py", line 188, in test_binary_op_scalar_fastpath
Jul 23 19:13:56     self._test_binary_op_scalar(device, dtype, op, N, scalar, True, disable_fastpath)
Jul 23 19:13:56   File "/var/lib/jenkins/workspace/test/test_foreach.py", line 169, in _test_binary_op_scalar
Jul 23 19:13:56     self._binary_test(dtype, op, ref, inputs, is_fastpath, is_inplace=False)
Jul 23 19:13:56   File "/var/lib/jenkins/workspace/test/test_foreach.py", line 107, in _binary_test
Jul 23 19:13:56     self.assertEqual(actual, expected)
Jul 23 19:13:56   File "/opt/conda/lib/python3.9/site-packages/torch/testing/_internal/common_utils.py", line 1601, in assertEqual
Jul 23 19:13:56     self.assertEqual(x_, y_, atol=atol, rtol=rtol, msg=msg,
Jul 23 19:13:56   File "/opt/conda/lib/python3.9/site-packages/torch/testing/_internal/common_utils.py", line 1533, in assertEqual
Jul 23 19:13:56     super().assertTrue(result, msg=self._get_assert_msg(msg, debug_msg=debug_msg))
Jul 23 19:13:56 AssertionError: False is not true : Tensors failed to compare as equal!Real parts failed to compare as equal! With rtol=1.3e-06 and atol=1e-05, found 1 element(s) (out of 256) whose difference(s) exceeded the margin of error (including 0 nan comparisons). The greatest difference was 1.2233853340148926e-05 (-0.2078128457069397 vs. -0.20780061185359955), which occurred at index (4, 14).
Jul 23 19:13:56 
Jul 23 19:13:56 ----------------------------------------------------------------------
Jul 23 19:13:56 Ran 1178 tests in 176.049s
Jul 23 19:13:56 
Jul 23 19:13:56 FAILED (failures=1)
Jul 23 19:13:56 
Jul 23 19:13:56 Generating XML reports...
Jul 23 19:13:56 Generated XML report: test-reports/python-unittest/test_foreach/TEST-TestForeachCUDA-20210723191100.xml
Jul 23 19:13:56 Traceback (most recent call last):
Jul 23 19:13:56   File "/var/lib/jenkins/workspace/test/run_test.py", line 1090, in <module>

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 · 2021-07-21T07:08:48Z

test/test_modules.py

+
+                # === Compare outputs to a reference if one is specified. ===
+                # TODO: Handle precision
+                reference_fn = module_input.reference_fn


Interesting that the reference_fn is on the module input

ModuleInput is possibly a bad name- it started out as a module analogue for SampleInput. We want the ability to have a reference function per (constructor args, forward args), so that was added there. I'm good with changing the name or whatever makes it less weird :p

Names can be sorted later, this seems fine

mruberry · 2021-07-21T07:10:34Z

torch/testing/_internal/common_modules.py

+        MODULE_CLASS_NAMES[module_cls] = f'{namespace_name}.{module_name}'
+
+
+class modules(_TestParametrizer):


Although "common_device_type.py" is an increasingly silly name, we might want this to go next to the ops decorator for clarity. Might as well add a comment, even if it's only one line:

PROTOTYPE @modules decorator that instantiates tests for each module

?

Hm, in an attempt to not increase the silliness of common_device_type.py further, I do prefer the @modules decorator being here. This is also the most obvious place I'd think to find it

mruberry · 2021-07-21T07:13:06Z

torch/testing/_internal/common_modules.py

+    module_inputs = [
+        ModuleInput(constructor_input=FunctionInput(10, 8),
+                    forward_input=FunctionInput(make_input((4, 10))),
+                    reference_fn=lambda m, p, i: torch.mm(i, p[0].t()) + p[1].view(1, -1).expand(4, 8)),


Interesting. In the future I wonder if the reference can just be a functional form of the module so you don't need to write a reference for each input?

The ability to write a reference per input is actually a feature. For a given (constructor args, forward args), the reference function may be as simple or complex as needed to verify the output. I think it'd be non-ideal to require that the module is essentially reimplemented within the reference, and if we just call the existing functional form of the module, that doesn't really test much. I'd also like to use the references from module_tests / new_module_tests / criterion_tests as much as possible, which is where these Linear ones came from.

Sure, pros/cons, and probably not too hard to change

torch/testing/_internal/common_modules.py

test/test_modules.py

mruberry · 2021-07-21T07:16:58Z

This is an awesome start; just need to fix lint and add the test file to run_tests.py

pytorch/test/run_test.py

Line 85 in db11619

'test_ops',

mruberry · 2021-07-23T05:36:01Z

FYI the moduleinfo test errors appear real but the polygamma test failures are in the base.

facebook-github-bot · 2021-07-23T18:12:10Z

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

mruberry

Cool! An exciting start to ModuleInfos

facebook-github-bot · 2021-07-27T14:43:26Z

@jbschlosser merged this pull request in a0309f8.

Summary: Follow up to #61935 This PR adds `test_pickle` to `test_modules`. Pull Request resolved: #63736 Reviewed By: heitorschueroff Differential Revision: D30522462 Pulled By: jbschlosser fbshipit-source-id: a03b66ea0d81c6d0845c4fddf0ddc3714bbf0ab1

Summary: Follow up to #61935 This PR adds `test_repr` to `test_modules`. Pull Request resolved: #63737 Reviewed By: gchanan Differential Revision: D30729642 Pulled By: jbschlosser fbshipit-source-id: c11a28bc0739abd3ed40727389dd28ed4069edad

Summary: Follow up to #61935 This PR adds inplace checks to `test_modules`. This version checks the constructor for `inplace` and performs the check automatically. Pull Request resolved: #63739 Reviewed By: saketh-are Differential Revision: D30737774 Pulled By: jbschlosser fbshipit-source-id: 8813534511e9296c8424d1ca878412726ddd4043

Summary: Follow up to #61935 cc albanD mruberry jbschlosser walterddr Pull Request resolved: #64444 Reviewed By: ngimel Differential Revision: D30867266 Pulled By: jbschlosser fbshipit-source-id: cbc0733261517dbfcdd3415d969b9e802b62b7ac

Summary: Follow up to #61935 cc albanD mruberry jbschlosser walterddr Pull Request resolved: #64444 Reviewed By: pbelevich Differential Revision: D31174672 Pulled By: jbschlosser fbshipit-source-id: 86dc3576479974fd0996f06298c09692c07e6b24

Summary: Follow up to #61935 This PR: 1. Adds test for non-contiguous tensors 2. Fixes bug in `NLLLoss` that was catch by the test. The reason this was not catch in `common_nn` is because `CriterionTest` overrides `test_cuda` but does not call `test_nonconfig`. cc albanD mruberry jbschlosser walterddr Pull Request resolved: #64954 Reviewed By: zou3519 Differential Revision: D31174149 Pulled By: jbschlosser fbshipit-source-id: a16073e59b40ccc01c82ede016b63a8db2e810f5

Summary: Follow up to #61935 This PR adds device to device transfer test into `ModuleInfo`. cc albanD mruberry jbschlosser walterddr Pull Request resolved: #65488 Reviewed By: mruberry Differential Revision: D32063662 Pulled By: jbschlosser fbshipit-source-id: 0868235a0ae7e5b6a3e4057c23fe70753c0946d2

Summary: Follow up to #61935 This PR: 1. Adds test for non-contiguous tensors 2. Fixes bug in `NLLLoss` that was catch by the test. The reason this was not catch in `common_nn` is because `CriterionTest` overrides `test_cuda` but does not call `test_nonconfig`. cc albanD mruberry jbschlosser walterddr Pull Request resolved: #64954 Reviewed By: zou3519 Differential Revision: D31174149 Pulled By: jbschlosser fbshipit-source-id: a16073e59b40ccc01c82ede016b63a8db2e810f5 (cherry picked from commit 0d3bf97) Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

) * TST Adds test for non-contiguous tensors (#64954) Summary: Follow up to #61935 This PR: 1. Adds test for non-contiguous tensors 2. Fixes bug in `NLLLoss` that was catch by the test. The reason this was not catch in `common_nn` is because `CriterionTest` overrides `test_cuda` but does not call `test_nonconfig`. cc albanD mruberry jbschlosser walterddr Pull Request resolved: #64954 Reviewed By: zou3519 Differential Revision: D31174149 Pulled By: jbschlosser fbshipit-source-id: a16073e59b40ccc01c82ede016b63a8db2e810f5 (cherry picked from commit 0d3bf97) Signed-off-by: Eli Uriegas <eliuriegas@fb.com> * Cherry-pick changes from #64444 Namely, `make_weight` partial into `module_inputs_torch_nn_NLLLoss` Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> Co-authored-by: Nikita Shulga <nshulga@fb.com>

jbschlosser requested a review from mruberry July 20, 2021 21:25

facebook-github-bot added the cla signed label Jul 20, 2021

mruberry reviewed Jul 21, 2021

View reviewed changes

torch/testing/_internal/common_modules.py Outdated Show resolved Hide resolved

mruberry reviewed Jul 21, 2021

View reviewed changes

test/test_modules.py Outdated Show resolved Hide resolved

jbschlosser added 4 commits July 23, 2021 14:11

Initial ModuleInfo implementation

8d57858

Address PR comments

437cb0b

Fix: lint / mypy

9c178b2

Fix default precision to get tests passing

3c4ecc8

jbschlosser requested a review from mruberry July 23, 2021 18:11

jbschlosser force-pushed the initial_module_info branch from 43bc84c to 3c4ecc8 Compare July 23, 2021 18:11

mruberry added the ci/master label Jul 24, 2021

mruberry approved these changes Jul 27, 2021

View reviewed changes

facebook-github-bot closed this in a0309f8 Jul 27, 2021

facebook-github-bot added the Merged label Jul 27, 2021

This was referenced Aug 22, 2021

TST Adds pickle testing for ModuleInfo #63736

Closed

TST Adds __repr__ and str to module info #63737

Closed

TST Adds reference_fn to module_info #63738

Closed

TST Adds inplace checks to module_info #63739

Closed

thomasjpfan mentioned this pull request Sep 2, 2021

TST Adds gradcheck and gradgradcheck to module info #64444

Closed

thomasjpfan mentioned this pull request Sep 8, 2021

TST Adds module test comparing output of cpu and gpu #64694

Closed

thomasjpfan mentioned this pull request Sep 13, 2021

TST Adds test for non-contiguous tensors #64954

Closed

This was referenced Sep 22, 2021

TST Adds device transfer into module info tests #65488

Closed

TST Sample based parameterization in ModuleTests #65562

Closed

jbschlosser mentioned this pull request Nov 8, 2021

Tracker: ModuleInfo-based testing #67913

Open

seemethere mentioned this pull request Dec 8, 2021

[release/1.10] TST Adds test for non-contiguous tensors (#64954) #69617

Merged

		MODULE_CLASS_NAMES[module_cls] = f'{namespace_name}.{module_name}'


		class modules(_TestParametrizer):

Initial ModuleInfo implementation #61935

Initial ModuleInfo implementation #61935

Uh oh!

Conversation

jbschlosser commented Jul 20, 2021

Uh oh!

facebook-github-bot commented Jul 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

💊 CI failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_linux_bionic_cuda10_2_cudnn7_py3_9_gcc7_test2 (1/1)

Uh oh!

mruberry Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

jbschlosser Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

mruberry Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

mruberry Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

jbschlosser Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

mruberry Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

mruberry Jul 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbschlosser Jul 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mruberry Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mruberry commented Jul 21, 2021

Uh oh!

mruberry commented Jul 23, 2021

Uh oh!

facebook-github-bot commented Jul 23, 2021

Uh oh!

mruberry left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jul 27, 2021

Uh oh!

Uh oh!

facebook-github-bot commented Jul 20, 2021 •

edited

Loading

mruberry Jul 21, 2021 •

edited

Loading

jbschlosser Jul 21, 2021 •

edited

Loading