Skip to content

[export] inline jit.scripted function in export #155180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from

Conversation

ydwu4
Copy link
Contributor

@ydwu4 ydwu4 commented Jun 5, 2025

Stack from ghstack (oldest at bottom):

When we export a scripted function, we inline the original callable stored in "_torchdynamo_inline", this is the same strategy as torch.compile path.

We do the same thing for script method, where a "__wrapped__" attribute points to the original callable in most cases. There are some corner cases we identified: top-level jit.scripted modules' method doesn't have a __wrapped__. In this case, we fall back to the original scripted approach. Maybe there're more such cases but need verification.

cc @EikanWang @jgong5 @wenzhe-nrv @sanchitintel

ydwu4 added a commit that referenced this pull request Jun 5, 2025
ghstack-source-id: 8c3efca
Pull Request resolved: #155180
Copy link

pytorch-bot bot commented Jun 5, 2025

🔗 Helpful Links

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

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 503740c with merge base 3f34d26 (image):

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 pytorch-bot bot added the release notes: jit release notes category label Jun 5, 2025
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jun 5, 2025
@ydwu4 ydwu4 added release notes: export export-triage-review This tag is used to tag issues that have been looked by PT2 Export team and pending discussions. and removed export-triage-review This tag is used to tag issues that have been looked by PT2 Export team and pending discussions. labels Jun 5, 2025
Copy link
Contributor

@tugsbayasgalan tugsbayasgalan left a comment

Choose a reason for hiding this comment

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

@justinchuby
Copy link
Collaborator

justinchuby commented Jun 5, 2025

cc: @justinchuby

Does this handle control flows of the scripted function? I thought it would be nice to directly inline the scripted function to leverage its ability to capture them (which is most of the time why a user would want to script the function in the first place)

cc @xadupre @titaiwangms

@ydwu4
Copy link
Contributor Author

ydwu4 commented Jun 5, 2025

Does this handle control flows of the scripted function

It handles the control flow the same way as we exporting the python code: we specialize the constants and shapes, errors out for data-dependent ones.

When we exporting a scripted function, we inline the original callable stored in "_torchdynamo_inline", this is the same strategy as torch.compile path. 

cc EikanWang jgong5 wenzhe-nrv sanchitintel

[ghstack-poisoned]
@justinchuby
Copy link
Collaborator

justinchuby commented Jun 5, 2025

Does this handle control flows of the scripted function

It handles the control flow the same way as we exporting the python code: we specialize the constants and shapes, errors out for data-dependent ones.

I think it can be more helpful: the scripted functions are usually data dependent with control flows (that’s often why they are used)

@ydwu4
Copy link
Contributor Author

ydwu4 commented Jun 5, 2025

Yeah, I agree. Maybe we could try to use the auto converter tool we're developing when we see a jit.script annotated function. It's also a good test bed for testing the coverage of the auto converter.

When we exporting a scripted function, we inline the original callable stored in "_torchdynamo_inline", this is the same strategy as torch.compile path. 

cc EikanWang jgong5 wenzhe-nrv sanchitintel

[ghstack-poisoned]
When we export a scripted function, we inline the original callable stored in "_torchdynamo_inline", this is the same strategy as torch.compile path. 

We do the same thing for script method, where a "\_\_wrapped\_\_" attribute points to the original callable in most cases. There are some corner cases we identified: top-level jit.scripted modules' method doesn't have a \_\_wrapped\_\_. In this case, we fall back to the original scripted approach. Maybe there're more such cases but need verification.

cc EikanWang jgong5 wenzhe-nrv sanchitintel

[ghstack-poisoned]
When we export a scripted function, we inline the original callable stored in "_torchdynamo_inline", this is the same strategy as torch.compile path. 

We do the same thing for script method, where a "\_\_wrapped\_\_" attribute points to the original callable in most cases. There are some corner cases we identified: top-level jit.scripted modules' method doesn't have a \_\_wrapped\_\_. In this case, we fall back to the original scripted approach. Maybe there're more such cases but need verification.

cc EikanWang jgong5 wenzhe-nrv sanchitintel

[ghstack-poisoned]
When we export a scripted function, we inline the original callable stored in "_torchdynamo_inline", this is the same strategy as torch.compile path. 

We do the same thing for script method, where a "\_\_wrapped\_\_" attribute points to the original callable in most cases. There are some corner cases we identified: top-level jit.scripted modules' method doesn't have a \_\_wrapped\_\_. In this case, we fall back to the original scripted approach. Maybe there're more such cases but need verification.

cc EikanWang jgong5 wenzhe-nrv sanchitintel

[ghstack-poisoned]
ydwu4 added a commit that referenced this pull request Jun 5, 2025
ghstack-source-id: 4f7e07d
Pull Request resolved: #155180
When we export a scripted function, we inline the original callable stored in "_torchdynamo_inline", this is the same strategy as torch.compile path. 

We do the same thing for script method, where a "\_\_wrapped\_\_" attribute points to the original callable in most cases. There are some corner cases we identified: top-level jit.scripted modules' method doesn't have a \_\_wrapped\_\_. In this case, we fall back to the original scripted approach. Maybe there're more such cases but need verification.

cc EikanWang jgong5 wenzhe-nrv sanchitintel

[ghstack-poisoned]
ydwu4 added a commit that referenced this pull request Jun 6, 2025
ghstack-source-id: b5912a0
Pull Request resolved: #155180
@ydwu4 ydwu4 added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 6, 2025
@ydwu4
Copy link
Contributor Author

ydwu4 commented Jun 10, 2025

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

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

thatgeeman pushed a commit to thatgeeman/pytorch-docathon that referenced this pull request Jun 15, 2025
When we export a scripted function, we inline the original callable stored in "_torchdynamo_inline", this is the same strategy as torch.compile path.

We do the same thing for script method, where a "\_\_wrapped\_\_" attribute points to the original callable in most cases. There are some corner cases we identified: top-level jit.scripted modules' method doesn't have a \_\_wrapped\_\_. In this case, we fall back to the original scripted approach. Maybe there're more such cases but need verification.

Pull Request resolved: pytorch#155180
Approved by: https://github.com/zou3519
@github-actions github-actions bot deleted the gh/ydwu4/257/head branch July 13, 2025 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged oncall: jit Add this issue/PR to JIT oncall triage queue release notes: export release notes: jit release notes category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants