Fix duplicated sources in inductor provenance tracking #159484
Conversation
Summary: Reduce compilation time when flag is on by at least 10x. Test Plan: ``` python test/higher_order_ops/test_invoke_subgraph.py -k test_input_mutation_mutiple_times ``` Rollback Plan: Differential Revision: D79203058
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/159484
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (6 Unrelated Failures)As of commit 48cf0e4 with merge base ee343ce ( BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
pytorch-bot
bot
added
ciflow/inductor
module: inductor
release notes: fx
|
This pull request was exported from Phabricator. Differential Revision: D79203058 |
yushangdi
changed the title
yushangdi
added
topic: not user facing
|
@pytorchbot merge -i |
Merge startedYour change will be merged while ignoring the following 6 checks: pull / linux-jammy-py3_9-clang9-xla / test (xla, 1, 1, lf.linux.12xlarge, unstable), pull / linux-jammy-cuda12.8-py3.10-gcc11-test / test (distributed, 2, 3, lf.linux.g4dn.12xlarge.nvidia.gpu), inductor / unit-test / cuda12.8-py3.10-gcc9-sm86 / test (inductor, 1, 2, linux.g5.4xlarge.nvidia.gpu), inductor / unit-test / cuda12.8-py3.10-gcc9-sm86 / test (inductor_distributed, 1, 1, linux.g5.12xlarge.nvidia.gpu), inductor / unit-test / cuda12.8-py3.12-gcc9-sm86 / test (inductor, 1, 2, linux.g5.4xlarge.nvidia.gpu), inductor / unit-test / cuda12.8-py3.13-gcc9-sm86 / test (inductor, 1, 2, linux.g5.4xlarge.nvidia.gpu) Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Summary:
The `replace_hook` is called once for each user of the replaced node. This fix avoids adding duplicated node sources.
This also means that if there are two nested pass like:
```
with GraphTransformObserver(gm, "outer"):
with GraphTransformObserver(gm, "inner"):
.....
```
We'll only see the outer pass's pass name recorded for the replaced node in the "from_node" node meta. I think this is fine. In practice, the outer pass usually contains a more meaningful name, e.g. `decompose_auto_functionalized`, and the inner pass name is just a default pass name like `pattern_matcher`.
Test Plan:
```
buck2 run @mode/dev-nosan fbcode//caffe2/test:fx -- -r test_graph_transform_observer_replace
```
Rollback Plan:
Differential Revision: D79203058
Pull Request resolved: #159484
Approved by: https://github.com/angelayi
4 participants
Summary:
The
replace_hookis called once for each user of the replaced node. This fix avoids adding duplicated node sources.This also means that if there are two nested pass like:
We'll only see the outer pass's pass name recorded for the replaced node in the "from_node" node meta. I think this is fine. In practice, the outer pass usually contains a more meaningful name, e.g.
decompose_auto_functionalized, and the inner pass name is just a default pass name likepattern_matcher.Test Plan:
Rollback Plan:
Differential Revision: D79203058
cc @ezyang @SherlockNoMad @EikanWang @jgong5 @wenzhe-nrv @voznesenskym @penguinwu @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @jiayisunx @ipiszy @chenyang78 @kadeng @muchulee8 @amjames @chauhang @aakhundov @coconutruben