Unify cache disable and cache bypass paths #141685

ezyang · 2024-11-27T14:03:55Z

Stack from ghstack (oldest at bottom):

I was constantly annoyed at the fact that we had a separate else branch for when cache was disabled which was distinct from when cache was bypassed. This diff gets rid of the disabled cache branch, so we use the same logic for bypass/disable. I actually think this change probably didn't actually matter much for the POC but I think it's cleaner.

Signed-off-by: Edward Z. Yang ezyang@meta.com

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang @aakhundov

[ghstack-poisoned]

pytorch-bot · 2024-11-27T14:04:01Z

🔗 Helpful Links

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

📄 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! (2 Unrelated Failures)

As of commit 5dcfc5a with merge base 0f261e8 ():

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 (inductor_timm, 1, 2, linux.g5.4xlarge.nvidia.gpu, unstable) (gh) (#141703)
convnext_base
inductor / cuda12.4-py3.10-gcc9-sm86 / test (inductor_timm, 1, 2, linux.g5.4xlarge.nvidia.gpu, unstable) (gh) (#141498)
convnext_base

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

[ghstack-poisoned]

jamesjwu

I'm having some trouble following the intention of this one. What does it really mean here to "unify" bypass and miss, if you still have two separate branches for them? I.e what's the end goal?

jamesjwu · 2024-11-27T16:00:19Z

torch/_inductor/compile_fx.py

-                    input._is_inductor_static = True  # type: ignore[attr-defined]
+        # TODO: This is a hack purely to get some info to extract_tensor_metadata_for_cache_key,
+        # figure out how to not have to modify example inputs
+        for i, input in enumerate(example_inputs):


How come we can unconditionally do this now instead of before?

It is a behavior change, but I guessed it would be harmless because I audited the use sites and there are no "negative" usages (e.g., explicitly tests that this field is not defined). So I am guessing it is harmless to always have this populated. CI seems to agree.

jamesjwu · 2024-11-27T16:02:35Z

torch/_inductor/compile_fx.py


-            local = config.fx_graph_cache
-            remote = fx_graph_remote_cache
+        # TODO: Remove this short circuit once types are unified here


Ok wait, isn't the entire point of this refactor to unify this type? Or is there another PR incoming to do that?

This gets eliminated in #141695

Signed-off-by: Edward Z. Yang <ezyang@meta.com> ghstack-source-id: b3dbf6e Pull Request resolved: #141685

ezyang · 2024-11-27T21:01:18Z

torch/_inductor/compile_fx.py

-                # In that case, we don't need to run all post compilation steps, we just need
-                # to return the string directly.
-                return compiled_graph
-            compiled_graph.post_compile2(example_inputs, cudagraphs, gm)


Specifically, this else branch got deleted

[ghstack-poisoned]

ezyang · 2024-11-28T04:44:26Z

@pytorchbot merge -f "unrelated failures"

ezyang · 2024-11-28T04:44:50Z

still needs review

pytorchmergebot · 2024-11-28T04:46:21Z

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

pytorchmergebot · 2024-11-28T04:46:28Z

Merge failed

Reason: PR #141685 has not been reviewed yet

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

[ghstack-poisoned]

Stacked on top of #141685 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #141688 Approved by: https://github.com/Skylion007, https://github.com/jansel ghstack dependencies: #141681, #141683, #141685

Stacked on #141688 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #141689 Approved by: https://github.com/jansel ghstack dependencies: #141681, #141683, #141685, #141688

Stacked on #141689 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #141691 Approved by: https://github.com/jansel ghstack dependencies: #141681, #141683, #141685, #141688, #141689

Stacked on #141691 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #141695 Approved by: https://github.com/aorenste ghstack dependencies: #141681, #141683, #141685, #141688, #141689, #141691

I was constantly annoyed at the fact that we had a separate else branch for when cache was disabled which was distinct from when cache was bypassed. This diff gets rid of the disabled cache branch, so we use the same logic for bypass/disable. I actually think this change probably didn't actually matter much for the POC but I think it's cleaner. Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#141685 Approved by: https://github.com/aorenste ghstack dependencies: pytorch#141681, pytorch#141683

Stacked on top of pytorch#141685 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#141688 Approved by: https://github.com/Skylion007, https://github.com/jansel ghstack dependencies: pytorch#141681, pytorch#141683, pytorch#141685

Stacked on pytorch#141688 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#141689 Approved by: https://github.com/jansel ghstack dependencies: pytorch#141681, pytorch#141683, pytorch#141685, pytorch#141688

Stacked on pytorch#141689 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#141691 Approved by: https://github.com/jansel ghstack dependencies: pytorch#141681, pytorch#141683, pytorch#141685, pytorch#141688, pytorch#141689

Stacked on pytorch#141691 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#141695 Approved by: https://github.com/aorenste ghstack dependencies: pytorch#141681, pytorch#141683, pytorch#141685, pytorch#141688, pytorch#141689, pytorch#141691

Signed-off-by: Edward Z. Yang <ezyang@meta.com> ghstack-source-id: 2b93a01 Pull Request resolved: pytorch/pytorch#141685

Update

2543fcf

[ghstack-poisoned]

This was referenced Nov 27, 2024

Code motion CompiledFxGraph to a dedicated file #141654

Closed

Move post compile steps into post_compile1/post_compile2 method #141656

Closed

pytorch-bot bot added ciflow/inductor module: inductor labels Nov 27, 2024

This was referenced Nov 27, 2024

Inline FxGraphCache.load into its sole call site #141681

Closed

Hoist set_feature_use out of conditional, rename some variables #141683

Closed

github-actions bot requested review from albanD, antoniojkim, bdhirsh, miladm and SherlockNoMad November 27, 2024 14:04

Update

0916700

[ghstack-poisoned]

ezyang requested review from oulgen, jamesjwu and masnesral November 27, 2024 14:04

ezyang mentioned this pull request Nov 27, 2024

Hoist post_compile1 into fx_codegen_and_compile #141688

Closed

ezyang added the topic: not user facing topic category label Nov 27, 2024

jamesjwu reviewed Nov 27, 2024

View reviewed changes

ezyang added a commit that referenced this pull request Nov 27, 2024

Unify cache disable and cache bypass paths

e243230

Signed-off-by: Edward Z. Yang <ezyang@meta.com> ghstack-source-id: b3dbf6e Pull Request resolved: #141685

ezyang commented Nov 27, 2024

View reviewed changes

ezyang requested a review from jamesjwu November 27, 2024 21:02

Update

b664913

[ghstack-poisoned]

This was referenced Nov 28, 2024

Unify post_compile1 and CompiledFxGraph constructor #141689

Closed

Inline compile_to_fn at its only call site #141691

Closed

Introduce CompiledAOTI #141695

Closed

ezyang requested a review from jansel November 28, 2024 04:44

pytorchmergebot added the merging label Nov 28, 2024

pytorchmergebot removed the merging label Nov 28, 2024

Update

5dcfc5a

[ghstack-poisoned]

ezyang mentioned this pull request Nov 28, 2024

[POC] AOTInductor as Inductor backend #141700

Closed

aorenste approved these changes Nov 28, 2024

View reviewed changes

pytorchmergebot closed this in cf3daf7 Nov 29, 2024

pytorchmergebot added the Merged label Nov 29, 2024

github-actions bot deleted the gh/ezyang/3025/head branch December 30, 2024 02:08

desai0007 pushed a commit to desai0007/test-repo-pytorch that referenced this pull request Feb 26, 2025

Unify cache disable and cache bypass paths

ada0144

Signed-off-by: Edward Z. Yang <ezyang@meta.com> ghstack-source-id: 2b93a01 Pull Request resolved: pytorch/pytorch#141685

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Unify cache disable and cache bypass paths #141685

Unify cache disable and cache bypass paths #141685

Uh oh!

ezyang commented Nov 27, 2024 •

edited

Loading

Uh oh!

pytorch-bot bot commented Nov 27, 2024 •

edited

Loading

Uh oh!

jamesjwu left a comment

Uh oh!

jamesjwu Nov 27, 2024

Uh oh!

ezyang Nov 27, 2024

Uh oh!

jamesjwu Nov 27, 2024

Uh oh!

ezyang Nov 27, 2024

Uh oh!

ezyang Nov 27, 2024

Uh oh!

ezyang commented Nov 28, 2024

Uh oh!

ezyang commented Nov 28, 2024

Uh oh!

pytorchmergebot commented Nov 28, 2024

Uh oh!

pytorchmergebot commented Nov 28, 2024

Uh oh!

Uh oh!

Unify cache disable and cache bypass paths #141685

Unify cache disable and cache bypass paths #141685

Uh oh!

Conversation

ezyang commented Nov 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Nov 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ You can merge normally! (2 Unrelated Failures)

Uh oh!

jamesjwu left a comment

Choose a reason for hiding this comment

Uh oh!

jamesjwu Nov 27, 2024

Choose a reason for hiding this comment

Uh oh!

ezyang Nov 27, 2024

Choose a reason for hiding this comment

Uh oh!

jamesjwu Nov 27, 2024

Choose a reason for hiding this comment

Uh oh!

ezyang Nov 27, 2024

Choose a reason for hiding this comment

Uh oh!

ezyang Nov 27, 2024

Choose a reason for hiding this comment

Uh oh!

ezyang commented Nov 28, 2024

Uh oh!

ezyang commented Nov 28, 2024

Uh oh!

pytorchmergebot commented Nov 28, 2024

Merge started

Uh oh!

pytorchmergebot commented Nov 28, 2024

Merge failed

Uh oh!

Uh oh!

ezyang commented Nov 27, 2024 •

edited

Loading

pytorch-bot bot commented Nov 27, 2024 •

edited

Loading