Merge torchdim into functorch build#82454
Merge torchdim into functorch build#82454zdevito wants to merge 14 commits intogh/zdevito/185/basefrom
Conversation
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
🔗 Helpful links
✅ No Failures (0 Pending)As of commit 5fa0348 (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. |
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. ghstack-source-id: 88db6ec Pull Request resolved: #82454
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. ghstack-source-id: b49f2b7 Pull Request resolved: #82454
| from .tree_map import tree_flatten, tree_map | ||
| from .wrap_type import wrap_type | ||
| from functorch._C import dim as _C | ||
| _C._patch_tensor_class() |
There was a problem hiding this comment.
To check my understanding: the monkey patching for torchdim happens on import functorch.dim or from functorch.dim import ... and does not happen if someone just calls import functorch, right?
There was a problem hiding this comment.
Correct, I was careful that no patching happens unless the dim package is imported. A follow up is to either write a more in-depth __torch_function__ for getitem/setitem, or to simply merge dim handling into getitem/setitem code.
|
Test failures are legit |
| print(name, elapsed / r) | ||
| return elapsed / r | ||
|
|
||
| class TestMin(TestCase): |
There was a problem hiding this comment.
For a test to be runnable via the pytorch testing infrastructure (i.e. run_test.py), it needs to extend from torch.testing._internal.common_utils.TestCase. That might just be a drop-in replacement for unittest.TestCase here. This is what the cause of the test failures are
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. ghstack-source-id: 7d08c57 Pull Request resolved: #82454
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. ghstack-source-id: 0cf9bad Pull Request resolved: #82454
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. ghstack-source-id: 1a3daf1 Pull Request resolved: #82454
|
@pytorchbot merge |
|
@pytorchbot merge |
|
@pytorchbot successfully started a merge and created land time checks. See merge status here and land check progress here |
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. Pull Request resolved: #82454 Approved by: https://github.com/ezyang, https://github.com/zou3519
|
@pytorchbot rebase |
|
@pytorchbot successfully started a rebase job. Check the current status here |
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
|
Successfully rebased |
|
@pytorchbot rebase |
|
@pytorchbot successfully started a rebase job. Check the current status here |
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
|
Successfully rebased |
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
|
@pytorchbot rebase |
|
@pytorchbot successfully started a rebase job. Check the current status here |
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
|
Successfully rebased |
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. [ghstack-poisoned]
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. ghstack-source-id: f5cd294 Pull Request resolved: #82454
|
@pytorchbot merge |
|
@pytorchbot successfully started a merge job. Check the current status here |
|
Hey @zdevito. |
Summary: This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. X-link: pytorch/pytorch#82454 Approved by: https://github.com/ezyang, https://github.com/zou3519 Reviewed By: seemethere Differential Revision: D38542272 Pulled By: zdevito fbshipit-source-id: 549eedf7aa0d3461774a1755235501d94bb5cc4b
Summary: This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. Pull Request resolved: #82454 Approved by: https://github.com/ezyang, https://github.com/zou3519 Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/d7cebab7d8ff4e5c5af271dc71b5181d368ad3f9 Reviewed By: seemethere Differential Revision: D38542272 Pulled By: zdevito fbshipit-source-id: 549eedf7aa0d3461774a1755235501d94bb5cc4b
Test Plan: revert-hammer Differential Revision: D38542272 Original commit changeset: 549eedf7aa0d Original Phabricator Diff: D38542272 fbshipit-source-id: 82cd1e373c2adb80445fd5da1ccc35f8fdfa96b7
…)" Summary: Original commit changeset: 82cd1e373c2a Original Phabricator Diff: D38542272 bypass-github-export-checks This diff didn't actually get reverted in Github so we can bypass the github export checks Test Plan: sandcastle Reviewed By: malfet, bigfootjon Differential Revision: D38583607 fbshipit-source-id: a674e0471e308d0bc90a0425f0ce8f09d764de5c
| const at::Tensor& operator*() const { | ||
| return *(at::Tensor*)this; | ||
| } | ||
| at::Tensor* operator->() const { | ||
| return (at::Tensor*)this; | ||
| } |
There was a problem hiding this comment.
these casts have undefined behavior, and because of TBAA I wouldn't be surprised at all if you were actually bitten by this in practice.
see the aliasing rules at https://en.cppreference.com/w/cpp/language/reinterpret_cast and note that C-style casts are interpreted as whichever fancy_cast matches, which will be reinterpret_cast in this case https://en.cppreference.com/w/cpp/language/explicit_cast
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. Pull Request resolved: pytorch#82454 Approved by: https://github.com/ezyang, https://github.com/zou3519
This reverts commit 1b3ab0c. Reverted pytorch#82454 on behalf of https://github.com/zengk95 due to this is breaking mac jobs on trunk https://hud.pytorch.org/pytorch/pytorch/commit/1b3ab0c1e2530d04aacd8f770edae92bd77d7bab
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim into the functorch build. This makes them availiable for use in PrimTorch more easily. Pull Request resolved: pytorch#82454 Approved by: https://github.com/ezyang, https://github.com/zou3519
Stack from ghstack (oldest at bottom):
This moves first-class dimensions, as prototyped in https://github.com/facebookresearch/torchdim
into the functorch build. This makes them availiable for use in PrimTorch more easily.