8000 BC fix for AOTIModelPackageLoader() constructor defaults by jbschlosser · Pull Request #149082 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

BC fix for AOTIModelPackageLoader() constructor defaults #149082

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

BC fix for AOTIModelPackageLoader() constructor defaults #149082

wants to merge 3 commits into from

Conversation

jbschlosser
Copy link
Contributor
@jbschlosser jbschlosser commented Mar 12, 2025 •
edited
Loading

Stack from ghstack (oldest at bottom):

The default value for run_single_threaded was wrongly specified in the .cpp file instead of the header, breaking C++-side instantiation of AOTIModelPackageLoader with no arguments. This PR fixes this and adds a test for the use case of running with AOTIModelPackageLoader instead of AOTIModelContainerRunner on the C++ side.

cc @desertfire @chenyang78 @penguinwu @yushangdi @benjaminglass1

@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Mar 12, 2025 •
edited
Loading

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit c6d633b with merge base b90698f (image):

NEW FAILURE - The following job has failed:

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

@jbschlosser jbschlosser requested a review from desertfire March 12, 2025 21:44
@jbschlosser
Update on "BC fix for AOTIModelPackageLoader() constructor defaults"
a752925 
The default value for `run_single_threaded` was wrongly specified in the .cpp file instead of the header, breaking C++-side instantiation of `AOTIModelPackageLoader` with no arguments. This PR fixes this and adds a test for the use case of running with `AOTIModelPackageLoader` instead of `AOTIMo
8000
delContainerRunner` on the C++ side.

cc desertfire chenyang78 penguinwu yushangdi benjaminglass1

[ghstack-poisoned]
desertfire
desertfire reviewed Mar 12, 2025
View reviewed changes
@jbschlosser
Update on "BC fix for AOTIModelPackageLoader() constructor defaults"
c6d633b 
The default value for `run_single_threaded` was wrongly specified in the .cpp file instead of the header, breaking C++-side instantiation of `AOTIModelPackageLoader` with no arguments. This PR fixes this and adds a test for the use case of running with `AOTIModelPackageLoader` instead of `AOTIModelContainerRunner` on the C++ side.

cc desertfire chenyang78 penguinwu yushangdi benjaminglass1

[ghstack-poisoned]
jbschlosser added a commit that referenced this pull request Mar 13, 2025
@jbschlosser
BC fix for AOTIModelPackageLoader() constructor defaults
9981a3e 
ghstack-source-id: 940a9c4
Pull Request resolved: #149082
@desertfire
Copy link
Contributor

xref #148601

@jbschlosser
< 8000 /summary> Copy link
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Mar 13, 2025
@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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: inductor / cuda12.6-py3.10-gcc9-sm86 / test (inductor_huggingface, 1, 1, linux.g5.4xlarge.nvidia.gpu)

Details for Dev Infra team Raised by workflow job

@jbschlosser
Copy link
Contributor Author

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 1 checks: inductor / cuda12.6-py3.10-gcc9-sm86 / test (inductor_huggingface, 1, 1, linux.g5.4xlarge.nvidia.gpu)

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

@etaf
Copy link
Collaborator
etaf commented Mar 14, 2025

Hi, @jbschlosser , @desertfire should we cherry-pick this fix to release 2.7? Without this fix, the example code in https://github.com/pytorch/pytorch/blob/main/docs/source/torch.compiler_aot_inductor.rst is getting compilation error:

[ 33%] Building CXX object CMakeFiles/aoti_example.dir/inference.cpp.o
/data/xinanlin/aoti-example/inference.cpp: In function ‘int main()’:
/data/xinanlin/aoti-example/inference.cpp:10:63: error: no matching function for call to ‘torch::inductor::AOTIModelPackageLoader::AOTIModelPackageLoader(const char [10])’
   10 |     torch::inductor::AOTIModelPackageLoader loader("model.pt2");

@jbschlosser
Copy link
Contributor Author

@pytorchbot cherry-pick --onto release/2.7 -c regression

@jbschlosser
Copy link
Contributor Author

@etaf yes that's the plan :)

pytorchbot pushed a commit that referenced this pull request Mar 14, 2025
@jbschlosser @pytorchbot
BC fix for AOTIModelPackageLoader() constructor defaults (#149082)
1e24e62 
The default value for `run_single_threaded` was wrongly specified in the .cpp file instead of the header, breaking C++-side instantiation of `AOTIModelPackageLoader` with no arguments. This PR fixes this and adds a test for the use case of running with `AOTIModelPackageLoader` instead of `AOTIModelContainerRunner` on the C++ side.

Pull Request resolved: #149082
Approved by: https://github.com/desertfire

(cherry picked from commit 5e1b715)
@pytorchbot pytorchbot mentioned this pull request Mar 14, 2025
Merged
@pytorchbot
Copy link
Collaborator

Cherry picking #149082

The cherry pick PR is at #149214 and it is recommended to link a regression cherry pick PR with an issue. The following tracker issues are updated:

Details for Dev Infra team Raised by workflow job

@pytorchbot pytorchbot mentioned this pull request Mar 14, 2025
Closed
ZainRizvi pushed a commit that referenced this pull request Mar 17, 2025
@pytorchbot @jbschlosser
BC fix for AOTIModelPackageLoader() constructor defaults (#149214)
6926f30 
BC fix for AOTIModelPackageLoader() constructor defaults (#149082)

The default value for `run_single_threaded` was wrongly specified in the .cpp file instead of the header, breaking C++-side instantiation of `AOTIModelPackageLoader` with no arguments. This PR fixes this and adds a test for the use case of running with `AOTIModelPackageLoader` instead of `AOTIModelContainerRunner` on the C++ side.

Pull Request resolved: #149082
Approved by: https://github.com/desertfire

(cherry picked from commit 5e1b715)

Co-authored-by: Joel Schlosser <jbschlosser@meta.com>
@github-actions github-actions bot deleted the gh/jbschlosser/231/head branch April 20, 2025 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@desertfire desertfire desertfire approved these changes

Assignees
No one assigned
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: aotinductor aot inductor release notes: inductor topic: bug fixes topic category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jbschlosser @desertfire @pytorchmergebot @etaf @pytorchbot
0