Modernize LoggingTensorMode #77667

soulitzer · 2022-05-17T18:29:41Z

This PR:

updates LoggingTensorMode to use the new PythonDispatchMode
updates some tests that use LoggingTensorMode to properly use the new version

Open questions:

why is the old python mode still working? Do we intend to keep it? (a small bit of additional logic is necessary to support it)
- This PR updates some of the tests that test old python mode behavior (e.g., all outputs are wrapped). If we intend to support old python mode long term we should bring it back, and just add new tests instead.

TODO (this PR):

Test nesting behavior

Stack from ghstack:

See #77544

[ghstack-poisoned]

ghstack-source-id: 9a73890 Pull Request resolved: #77667

facebook-github-bot · 2022-05-17T18:29:47Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/77667
📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓Need help or want to give feedback on the CI? Visit our office hours
↩️ [fb-only] Re-run with SSH instructions

❌ 2 New Failures

As of commit 60b4ec8 (more details on the Dr. CI page):

Expand to see more

2/2 failures introduced in this PR

🕵️ 2 new failures recognized by patterns

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

pull / linux-xenial-py3.7-gcc5.4 / test (backwards_compat, 1, 1, linux.2xlarge) (1/2)

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

2022-05-24T01:37:20.0827200Z The PR is introduc...m to confirm whether this change is wanted or not.

2022-05-24T01:37:20.0813984Z processing existing schema:  text(__torch__.torch.classes.profiling.SourceRef _0) -> (str _0)
2022-05-24T01:37:20.0815304Z processing existing schema:  count(__torch__.torch.classes.profiling.InstructionStats _0) -> (int _0)
2022-05-24T01:37:20.0816408Z processing existing schema:  duration_ns(__torch__.torch.classes.profiling.InstructionStats _0) -> (int _0)
2022-05-24T01:37:20.0817833Z processing existing schema:  source(__torch__.torch.classes.profiling.SourceStats _0) -> (__torch__.torch.classes.profiling.SourceRef _0)
2022-05-24T01:37:20.0819431Z processing existing schema:  line_map(__torch__.torch.classes.profiling.SourceStats _0) -> (Dict(int, __torch__.torch.classes.profiling.InstructionStats) _0)
2022-05-24T01:37:20.0820769Z processing existing schema:  __init__(__torch__.torch.classes.profiling._ScriptProfile _0) -> (NoneType _0)
2022-05-24T01:37:20.0821460Z processing existing schema:  enable(__torch__.torch.classes.profiling._ScriptProfile _0) -> (NoneType _0)
2022-05-24T01:37:20.0822869Z processing existing schema:  disable(__torch__.torch.classes.profiling._ScriptProfile _0) -> (NoneType _0)
2022-05-24T01:37:20.0824454Z processing existing schema:  _dump_stats(__torch__.torch.classes.profiling._ScriptProfile _0) -> (__torch__.torch.classes.profiling.SourceStats[] _0)
2022-05-24T01:37:20.0826953Z processing existing schema:  __init__(__torch__.torch.classes.dist_rpc.WorkerInfo _0, str _1, int _2) -> (NoneType _0)
2022-05-24T01:37:20.0827200Z The PR is introducing backward incompatible changes to the operator library. Please contact PyTorch team to confirm whether this change is wanted or not. 
2022-05-24T01:37:20.0827221Z 
2022-05-24T01:37:20.0827294Z Broken ops: [
2022-05-24T01:37:20.0827541Z 	prims::uniform(int[] shape, *, Scalar low, Scalar high, int dtype, Device device) -> (Tensor)
2022-05-24T01:37:20.0827800Z 	prims::empty_strided(int[] shape, int[] strides, *, int dtype, Device device, bool requires_grad) -> (Tensor)
2022-05-24T01:37:20.0828113Z 	prims::var(Tensor inp, int[]? dims, *, int correction, int? output_dtype=None) -> (Tensor)
2022-05-24T01:37:20.0828288Z 	prims::where(Tensor pred, Tensor a, Tensor b) -> (Tensor)
2022-05-24T01:37:20.0828441Z 	prims::cat(Tensor[] tensors, int dim) -> (Tensor)
2022-05-24T01:37:20.0828578Z 	prims::log10(Tensor self) -> (Tensor)
2022-05-24T01:37:20.0828723Z 	prims::fill(Tensor self, Scalar value) -> (Tensor)
2022-05-24T01:37:20.0828901Z 	prims::exp2(Tensor self) -> (Tensor)

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

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

2022-05-24T02:22:07.3712395Z FAIL [10.421s]: test_fs_sharing (__main__.TestMultiprocessing)

2022-05-24T02:22:07.3439826Z   test_non_leaf_variable_sharing (__main__.TestMultiprocessing) ... ok (0.001s)
2022-05-24T02:22:07.3481301Z   test_parameter_sharing (__main__.TestMultiprocessing) ... /opt/conda/lib/python3.7/site-packages/torch/utils/hooks.py:62: UserWarning: backward hook <function TestMultiprocessing._test_autograd_sharing.<locals>.hook at 0x7f2b8fbd5e60> on tensor will not be serialized.  If this is expected, you can decorate the function with @torch.utils.hooks.unserializable_hook to suppress this warning
2022-05-24T02:22:07.3481971Z   "to suppress this warning".format(repr(hook)))
2022-05-24T02:22:07.3530162Z ok (0.009s)
2022-05-24T02:22:07.3574223Z   test_variable_sharing (__main__.TestMultiprocessing) ... /opt/conda/lib/python3.7/site-packages/torch/utils/hooks.py:62: UserWarning: backward hook <function TestMultiprocessing._test_autograd_sharing.<locals>.hook at 0x7f2b8fbb98c0> on tensor will not be serialized.  If this is expected, you can decorate the function with @torch.utils.hooks.unserializable_hook to suppress this warning
2022-05-24T02:22:07.3574878Z   "to suppress this warning".format(repr(hook)))
2022-05-24T02:22:07.3701362Z ok (0.017s)
2022-05-24T02:22:07.3711558Z   test_wrong_cuda_fork (__main__.TestMultiprocessing) ... skip: CUDA not available (0.001s)
2022-05-24T02:22:07.3711933Z 
2022-05-24T02:22:07.3712054Z ======================================================================
2022-05-24T02:22:07.3712395Z FAIL [10.421s]: test_fs_sharing (__main__.TestMultiprocessing)
2022-05-24T02:22:07.3713162Z ----------------------------------------------------------------------
2022-05-24T02:22:07.3713431Z Traceback (most recent call last):
2022-05-24T02:22:07.3713702Z   File "test_multiprocessing.py", line 347, in test_fs_sharing
2022-05-24T02:22:07.3713972Z     self._test_sharing(repeat=TEST_REPEATS)
2022-05-24T02:22:07.3714226Z   File "test_multiprocessing.py", line 288, in _test_sharing
2022-05-24T02:22:07.3714457Z     test_fill()
2022-05-24T02:22:07.3714692Z   File "test_multiprocessing.py", line 258, in test_fill
2022-05-24T02:22:07.3715145Z     self.assertTrue(e.is_set())
2022-05-24T02:22:07.3715357Z AssertionError: False is not true
2022-05-24T02:22:07.3715557Z

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

See #77544 [ghstack-poisoned]

torch/utils/_mode_utils.py

This PR: - updates LoggingTensorMode to use the new PythonDispatchMode - updates some tests that use LoggingTensorMode to properly use the new version Open questions: - why is the old python mode still working? Do we intend to keep it? (a small bit of additional logic is necessary to support it) See #77544 [ghstack-poisoned]

ezyang · 2022-05-18T16:26:36Z

IMO we should just dump old python mode

soulitzer · 2022-05-18T16:29:47Z

torch/testing/_internal/logging_tensor.py

+
+@contextlib.contextmanager
+def capture_logs_with_logging_tensor_mode():
+    with push_torch_dispatch_mode(LoggingTensorMode(inner=None)), capture_logs(True) as logs:


TODO: shouldn't don't need inner

@samdow yeah this looks like we should just support direct constructor lol

We should also add a test for this function

I'm pretty sure that as written this will error for the inner reason. Also flags to me that I really dislike that the argument for push_torch_dispatch_mode is going to be a constructor for a Mode while enable_torch_dispatch_mode is going to be the Mode instance. TODO (@samdow): fix that

This PR: - updates LoggingTensorMode to use the new PythonDispatchMode - updates some tests that use LoggingTensorMode to properly use the new version Open questions: - why is the old python mode still working? Do we intend to keep it? (a small bit of additional logic is necessary to support it) - This PR updates some of the tests that test old python mode behavior (e.g., all outputs are wrapped). If we intend to support old python mode long term we should bring it back, and just add new tests instead. TODO (this PR): - Test nesting behavior See #77544 [ghstack-poisoned]

ghstack-source-id: 277d87e Pull Request resolved: #77667

This PR: - updates LoggingTensorMode to use the new PythonDispatchMode - updates some tests that use LoggingTensorMode to properly use the new version Open questions: - why is the old python mode still working? Do we intend to keep it? (a small bit of additional logic is necessary to support it) - This PR updates some of the tests that test old python mode behavior (e.g., all outputs are wrapped). If we intend to support old python mode long term we should bring it back, and just add new tests instead. TODO (this PR): - Test nesting behavior See #77544 [ghstack-poisoned]

ghstack-source-id: efe9d2f Pull Request resolved: #77667

torch/testing/_internal/logging_tensor.py

test/test_python_dispatch.py

This PR: - updates LoggingTensorMode to use the new PythonDispatchMode - updates some tests that use LoggingTensorMode to properly use the new version Open questions: - why is the old python mode still working? Do we intend to keep it? (a small bit of additional logic is necessary to support it) - This PR updates some of the tests that test old python mode behavior (e.g., all outputs are wrapped). If we intend to support old python mode long term we should bring it back, and just add new tests instead. TODO (this PR): - Test nesting behavior See #77544 [ghstack-poisoned]

ghstack-source-id: d2f3c7c Pull Request resolved: #77667

soulitzer · 2022-05-24T01:29:33Z

@pytorchbot merge this on green

pytorchmergebot · 2022-05-24T03:23:21Z

Merge failed due to Matched rule superuser, but PR has not been reviewed yet
Raised by https://github.com/pytorch/pytorch/actions/runs/2374910721

soulitzer · 2022-05-24T17:44:01Z

@pytorchbot merge this

pytorchmergebot · 2022-05-24T17:45:22Z

Merge failed due to Matched rule superuser, but PR has not been reviewed yet
Raised by https://github.com/pytorch/pytorch/actions/runs/2379606786

pytorchmergebot · 2022-05-24T20:47:50Z

Merge failed due to Matched rule superuser, but PR has not been reviewed yet
Raised by https://github.com/pytorch/pytorch/actions/runs/2379606786

When merging stack, it could be confusing to see which PRs are missing reviews as one can observe in #77667 (comment) Print PR number in needs-review message

soulitzer · 2022-05-24T22:40:34Z

@pytorchbot merge this

github-actions · 2022-05-24T22:51:00Z

Hey @soulitzer.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

When merging stack, it could be confusing to see which PRs are missing reviews as one can observe in #77667 (comment) Print PR number in needs-review message Pull Request resolved: #78219 Approved by: https://github.com/atalman, https://github.com/kit1980, https://github.com/seemethere

Summary: Pull Request resolved: #77667 Approved by: https://github.com/malfet Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/f3af51069d758bee2a68f35ae9491a2e13f7b63e Reviewed By: mehtanirav Differential Revision: D36668817 Pulled By: soulitzer fbshipit-source-id: 0547ef32f770ee82abf4a04b3d8fb9943c97635f

Summary: When merging stack, it could be confusing to see which PRs are missing reviews as one can observe in #77667 (comment) Print PR number in needs-review message Pull Request resolved: #78219 Approved by: https://github.com/atalman, https://github.com/kit1980, https://github.com/seemethere Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/357707b9f9b09eef2e50d2ba035f1c694be6018e Reviewed By: mehtanirav Differential Revision: D36668860 Pulled By: malfet fbshipit-source-id: fb08f0b747f75ef4bdd9d49a52a33b7ed0022877

Modernize LoggingTensorMode

0e0bce9

[ghstack-poisoned]

facebook-github-bot added the cla signed label May 17, 2022

soulitzer added a commit that referenced this pull request May 17, 2022

Modernize LoggingTensorMode

f54079c

ghstack-source-id: 9a73890 Pull Request resolved: #77667

soulitzer changed the title ~~Modernize LoggingTensorMode~~ [WIP] Modernize LoggingTensorMode May 17, 2022

Update on "[WIP] Modernize LoggingTensorMode"

d96ecc4

See #77544 [ghstack-poisoned]

soulitzer mentioned this pull request May 18, 2022

Fix slow gradcheck when outputs that don't require grad precede those that do #77743

Closed

soulitzer commented May 18, 2022

View reviewed changes

torch/utils/_mode_utils.py Outdated Show resolved Hide resolved

soulitzer changed the title ~~[WIP] Modernize LoggingTensorMode~~ Modernize LoggingTensorMode May 18, 2022

soulitzer requested a review from ezyang May 18, 2022 15:45

ezyang requested review from zou3519 and albanD May 18, 2022 16:26

soulitzer commented May 18, 2022

View reviewed changes

soulitzer added a commit that referenced this pull request May 18, 2022

Modernize LoggingTensorMode

438d1a6

ghstack-source-id: 277d87e Pull Request resolved: #77667

soulitzer added a commit that referenced this pull request May 18, 2022

Modernize LoggingTensorMode

4f13a18

ghstack-source-id: efe9d2f Pull Request resolved: #77667

ezyang approved these changes May 20, 2022

View reviewed changes

zou3519 reviewed May 20, 2022

View reviewed changes

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

zou3519 reviewed May 20, 2022

View reviewed changes

test/test_python_dispatch.py Outdated Show resolved Hide resolved

soulitzer added a commit that referenced this pull request May 24, 2022

Modernize LoggingTensorMode

112479c

ghstack-source-id: d2f3c7c Pull Request resolved: #77667

malfet mentioned this pull request May 24, 2022

[GHF] Better "Reviews missing" error message #78219

Closed

malfet added a commit that referenced this pull request May 24, 2022

[GHF] Better "Reviews missing" error message

1533d5d

When merging stack, it could be confusing to see which PRs are missing reviews as one can observe in #77667 (comment) Print PR number in needs-review message

pytorchmergebot added the Merged label May 24, 2022

pytorchmergebot closed this in f3af510 May 24, 2022

This was referenced May 26, 2022

[forward ad] Sync conj between primal and tangent on set forward grad #78358

Closed

[forward ad] forbid non-float non-complex tangent and primal #78361

Closed

facebook-github-bot deleted the gh/soulitzer/80/head branch May 28, 2022 14:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Modernize LoggingTensorMode #77667

Modernize LoggingTensorMode #77667

Uh oh!

soulitzer commented May 17, 2022 •

edited

Loading

Uh oh!

facebook-github-bot commented May 17, 2022 •

edited

Loading

🕵️ 2 new failures recognized by patterns

pull / linux-xenial-py3.7-gcc5.4 / test (backwards_compat, 1, 1, linux.2xlarge) (1/2)

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

Uh oh!

Uh oh!

ezyang commented May 18, 2022

Uh oh!

soulitzer May 18, 2022

Uh oh!

ezyang May 20, 2022

Uh oh!

samdow May 20, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

soulitzer commented May 24, 2022

Uh oh!

pytorchmergebot commented May 24, 2022

Uh oh!

soulitzer commented May 24, 2022

Uh oh!

pytorchmergebot commented May 24, 2022

Uh oh!

pytorchmergebot commented May 24, 2022

Uh oh!

soulitzer commented May 24, 2022

Uh oh!

github-actions bot commented May 24, 2022

Uh oh!

Uh oh!

Modernize LoggingTensorMode #77667

Modernize LoggingTensorMode #77667

Uh oh!

Conversation

soulitzer commented May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

❌ 2 New Failures

🕵️ 2 new failures recognized by patterns

pull / linux-xenial-py3.7-gcc5.4 / test (backwards_compat, 1, 1, linux.2xlarge) (1/2)

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

Uh oh!

Uh oh!

ezyang commented May 18, 2022

Uh oh!

soulitzer May 18, 2022

Choose a reason for hiding this comment

Uh oh!

ezyang May 20, 2022

Choose a reason for hiding this comment

Uh oh!

samdow May 20, 2022 • 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!

Uh oh!

Uh oh!

soulitzer commented May 24, 2022

Uh oh!

pytorchmergebot commented May 24, 2022

Uh oh!

soulitzer commented May 24, 2022

Uh oh!

pytorchmergebot commented May 24, 2022

Uh oh!

pytorchmergebot commented May 24, 2022

Uh oh!

soulitzer commented May 24, 2022

Uh oh!

github-actions bot commented May 24, 2022

Uh oh!

Uh oh!

soulitzer commented May 17, 2022 •

edited

Loading

facebook-github-bot commented May 17, 2022 •

edited

Loading

samdow May 20, 2022 •

edited

Loading