Beef up the allow_in_graph docs #127117

zou3519 · 2024-05-24T18:52:34Z

Stack from ghstack (oldest at bottom):

-> Beef up the allow_in_graph docs #127117

We make the following changes:

most of the time when someone uses allow_in_graph, they actually
wanted to make a custom op. We add a link to the custom ops landing
page and explain the differences between allow_in_graph and custom
ops.
we warn people against using allow_in_graph footguns and document
them.

Test Plan:

tests

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the Custom Operators Manual. You might notice it's a gdoc; our strategy for custom ops docs is a combination of tutorials (on pytorch/tutorials) and a single gdoc (so we can easily make changes) - we warn people against using allow_in_graph and document some of its restrictions: input/output types and lack of ability to handle closures. Test Plan: - tests [ghstack-poisoned]

pytorch-bot · 2024-05-24T18:52:37Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/127117

📄 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 the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (5 Unrelated Failures)

As of commit 7613947 with merge base 4644def ():

FLAKY - The following job failed but was likely due to flakiness present on trunk:

trunk / macos-13-py3-arm64 / build (gh) (detected as infra flaky with no runner)

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

inductor / cuda12.1-py3.10-gcc9-sm86 / test (dynamic_inductor_timm, 2, 2, linux.g5.4xlarge.nvidia.gpu, unstable) (gh) (#127438)
sebotnet33ts_256
inductor / cuda12.1-py3.10-gcc9-sm86 / test (inductor_timm, 1, 2, linux.g5.4xlarge.nvidia.gpu, unstable) (gh) (#126884)
cspdarknet53
inductor / linux-jammy-cpu-py3.8-gcc11-inductor / test (inductor_torchbench_cpu_smoketest_perf, 1, 1, linux.24xl.spr-metal, unstable) (gh) (#126993)
Process completed with exit code 1.
pull / linux-focal-cuda12.4-py3.10-gcc9-sm86 / test (default, 5, 5, linux.g5.4xlarge.nvidia.gpu, unstable) (gh) ()
inductor/test_efficient_conv_bn_eval.py::EfficientConvBNEvalCudaTests::test_basic_cuda

This comment was automatically generated by Dr. CI and updates every 15 minutes.

torch/_dynamo/decorators.py

torch/compiler/__init__.py

torch/_dynamo/decorators.py

zou3519 · 2024-05-28T20:23:54Z

Followups from chatting with @jansel

# (make issue) zero-tensor meta
# mutate args can be optional (and be pessimistic)
# raw pybind: use warning instead of graph break (because the graph break message gets suppressed)

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests [ghstack-poisoned]

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests ghstack-source-id: bbba3697312c4963438ed8b8e631c853e59737ca Pull Request resolved: #127117

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests [ghstack-poisoned]

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests ghstack-source-id: 33fb6ee68fa42876f5ee540ace572b5b4a426406 Pull Request resolved: #127117

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests [ghstack-poisoned]

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests ghstack-source-id: 8808ad63048fd0eee65e94c523774115b7752b2d Pull Request resolved: #127117

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests [ghstack-poisoned]

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests ghstack-source-id: 0e10a418eca1a1ab2947c8620ca431fd4a01b426 Pull Request resolved: #127117

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests [ghstack-poisoned]

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests ghstack-source-id: 22b872fbea85b1db35fd32d12f5b39c9e651781d Pull Request resolved: #127117

zou3519 · 2024-05-30T13:25:30Z

This should be ready for another pass through. I updated the PR with jansel's feedback and our offline discussion:

removed mentions of torch.fx.wrap, because it's currently confusing (allow_in_graph is a torch.fx.wrap for Dynamo, but not for export today (because export today is Dynamo + AOTAutograd)).
added more clarity on when to use allow_in_graph and when to use custom ops
the docs page we link to is now a website

albanD

Very clear comparison with custom ops. Thanks!

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests [ghstack-poisoned]

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests ghstack-source-id: 8de4e23606f66dfefd9e694e3f58e88097f98b72 Pull Request resolved: #127117

zou3519 · 2024-06-01T14:30:19Z

@pytorchbot merge

pytorchmergebot · 2024-06-01T14:32:28Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2024-06-01T20:31:11Z

The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command
For more information see pytorch-bot wiki.

jansel · 2024-06-01T20:58:25Z

@pytorchbot merge

pytorchmergebot · 2024-06-01T21:00:11Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2024-06-02T02:58:53Z

The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command
For more information see pytorch-bot wiki.

zou3519 · 2024-06-02T14:58:47Z

@pytorchbot merge -f "mergebot seems stuck"

pytorchmergebot · 2024-06-02T15:00:30Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

We make the following changes: - most of the time when someone uses allow_in_graph, they actually wanted to make a custom op. We add a link to the custom ops landing page and explain the differences between allow_in_graph and custom ops. - we warn people against using allow_in_graph footguns and document them. Test Plan: - tests Pull Request resolved: pytorch#127117 Approved by: https://github.com/jansel, https://github.com/albanD

pytorch-bot bot added ciflow/inductor module: dynamo labels May 24, 2024

This was referenced May 24, 2024

Add unsafe_allow_in_graph; deprecate allow_in_graph #127118

Closed

Change usages of allow_in_graph to unsafe_allow_in_graph #127119

Closed

zou3519 requested review from jansel and albanD May 24, 2024 19:34

zou3519 added the release notes: dynamo label May 24, 2024

jansel requested changes May 24, 2024

View reviewed changes

This was referenced May 29, 2024

New Custom Ops Documentation landing page #127400

Closed

Rewrite existing links to custom ops gdocs with the landing page #127423

Closed

Better graph break msg (and warning) on Dynamo x Python C++ extension #127301

Closed

zou3519 requested a review from jansel May 30, 2024 13:25

jansel approved these changes May 30, 2024

View reviewed changes

albanD approved these changes May 30, 2024

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 1, 2024

pytorchmergebot added the merging label Jun 1, 2024

pytorchmergebot closed this in 08653fe Jun 2, 2024

pytorchmergebot added Merged and removed merging labels Jun 2, 2024

github-actions bot deleted the gh/zou3519/990/head branch July 4, 2024 01:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Beef up the allow_in_graph docs #127117

Beef up the allow_in_graph docs #127117

zou3519 commented May 24, 2024 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented May 24, 2024 •

edited

Loading

zou3519 commented May 28, 2024 •

edited

Loading

zou3519 commented May 30, 2024

albanD left a comment

zou3519 commented Jun 1, 2024

pytorchmergebot commented Jun 1, 2024

pytorchmergebot commented Jun 1, 2024

jansel commented Jun 1, 2024

pytorchmergebot commented Jun 1, 2024

pytorchmergebot commented Jun 2, 2024

zou3519 commented Jun 2, 2024

pytorchmergebot commented Jun 2, 2024

Beef up the allow_in_graph docs #127117

Beef up the allow_in_graph docs #127117

Conversation

zou3519 commented May 24, 2024 • edited by pytorch-bot bot Loading

pytorch-bot bot commented May 24, 2024 • edited Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/127117

✅ You can merge normally! (5 Unrelated Failures)

zou3519 commented May 28, 2024 • edited Loading

zou3519 commented May 30, 2024

albanD left a comment

Choose a reason for hiding this comment

zou3519 commented Jun 1, 2024

pytorchmergebot commented Jun 1, 2024

Merge started

pytorchmergebot commented Jun 1, 2024

jansel commented Jun 1, 2024

pytorchmergebot commented Jun 1, 2024

Merge started

pytorchmergebot commented Jun 2, 2024

zou3519 commented Jun 2, 2024

pytorchmergebot commented Jun 2, 2024

Merge started

zou3519 commented May 24, 2024 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented May 24, 2024 •

edited

Loading

zou3519 commented May 28, 2024 •

edited

Loading