Skip to content

Conversation

@anijain2305
Copy link
Contributor

@anijain2305 anijain2305 commented Nov 7, 2025

Stack from ghstack (oldest at bottom):

PR Description and code-comments generated by claude

Summary

Refactors speculate_subgraph to decouple graph outputs from output VariableTrackers (VTs), enabling better support for higher-order operators (HOPs) with non-proxyable outputs.

Motivation

Previously, speculate_subgraph unnecessarily entangled the graph output and the call_function VTs that needed to be sent for later tracing. In Dynamo (outside of speculate subgraph), VTs and graph are separate concepts: VTs can run ahead while the graph is just a side data structure of computation seen so far.

This entanglement caused issues with HOP support where subgraph outputs are not proxyable. The old implementation would call pytree flatten because it was unnecessarily coupling graph_output and output VTs, leading to complications with non-tensor/non-proxyable returns.

Changes

Core Refactoring:

  • Removed the OutputSpec dataclass that tracked treespec, constant value masks, and filtering logic
  • Introduced graph_output_vts - a separate tuple of VTs that should be graph outputs
  • VTs are now collected by visiting the output structure and extracting only TensorVariable and SymNodeVariable instances
  • Graph outputs and function return values are now properly separated concepts

Key Benefits:

  1. Cleaner separation of concerns: Graph outputs (what goes in the FX graph) vs function outputs (what Dynamo continues tracing with)
  2. Better non-proxyable output support: Enables HOPs to return custom objects (UDFs) that contain tensors without forcing them to be graph outputs
  3. Simplified code: Removed complex logic for filtering constants and maintaining treespecs
  4. Maintains proxy overwriting: Still correctly overwrites original VT proxies with subgraph output proxies for proper graph construction

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

[ghstack-poisoned]
@anijain2305 anijain2305 requested a review from zou3519 as a code owner November 7, 2025 22:49
@pytorch-bot
Copy link

pytorch-bot bot commented Nov 7, 2025

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 4325dd1 with merge base 47acdea (image):

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

[ghstack-poisoned]
anijain2305 added a commit that referenced this pull request Nov 7, 2025
[ghstack-poisoned]
anijain2305 added a commit that referenced this pull request Nov 8, 2025
[ghstack-poisoned]
anijain2305 added a commit that referenced this pull request Nov 8, 2025
[ghstack-poisoned]
anijain2305 added a commit that referenced this pull request Nov 8, 2025
@anijain2305 anijain2305 added ciflow/trunk Trigger trunk jobs on your pull request topic: not user facing topic category labels Nov 8, 2025
[ghstack-poisoned]
anijain2305 added a commit that referenced this pull request Nov 8, 2025
[ghstack-poisoned]
anijain2305 added a commit that referenced this pull request Nov 8, 2025
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@anijain2305 anijain2305 added the keep-going Don't stop on first failure, keep running tests until the end label Nov 9, 2025
[ghstack-poisoned]
Comment on lines +2524 to +2526
backend = AotEagerAndRecordGraphs()

opt_fn = torch.compile(fn, backend=backend, fullgraph=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Use aot_eager if you're not going to do anything with the graphs :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request keep-going Don't stop on first failure, keep running tests until the end module: dynamo topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants