[AOTInductor] Include constants in AOTInductor .so file. #107718

muchulee8 · 2023-08-22T18:13:00Z

Summary:
Include the constants into AOTInductor .so file.
We do not modify existing API signatures but create necessary format with weight lifted out instead.

Test Plan:
test/inductor/test_aot_inductor.py

Reviewers:

Subscribers:

Tasks:

Tags:

Fixes #ISSUE_NUMBER

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @ngimel @yf225 @chenyang78 @kadeng @aakhundov @anijain2305

Marking as already reverted:
@diff-train-skip-merge
see cf64a9e

pytorch-bot · 2023-08-22T18:13:02Z

🔗 Helpful Links

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

📄 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.

⏳ No Failures, 2 Pending

As of commit b6e8801 with merge base c85c595 ():
💚 Looks good so far! There are no failures yet. 💚

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

linux-foundation-easycla · 2023-08-22T18:13:03Z

The committers listed above are authorized under a signed CLA.

✅ login: muchulee8 / name: Mu-Chu Lee (decdc45, d457331, b6e8801)

facebook-github-bot · 2023-08-22T18:15:01Z

@muchulee8 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2023-08-22T19:11:42Z

@muchulee8 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2023-08-22T19:17:48Z

@muchulee8 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2023-08-22T20:09:01Z

@muchulee8 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

angelayi

AOT side generally looks fine to me! Do you mind including a paste of what the generated file looks like?

torch/_export/__init__.py

torch/_inductor/codegen/wrapper.py

torch/_inductor/compile_fx.py

torch/_inductor/config.py

torch/_inductor/codegen/wrapper.py

eellison

Would you mind separating out the formatting changes or removing them ? (ghstack is nice for this)

facebook-github-bot · 2023-08-23T23:30:26Z

@muchulee8 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

torch/_inductor/codecache.py

torch/_inductor/codegen/wrapper.py

torch/_inductor/graph.py

torch/csrc/inductor/aot_inductor_model_container.h

eellison

Cool ! did not review codecache. will defer to @desertfire on some of the code organization.

It might be easier and more general to write the entire untyped_storage() of tensor constant, that would both handle striding and offsets, and allow you to dedup tensors with shared storages.

eellison · 2023-08-24T17:44:35Z

torch/_inductor/config.py

 aot_inductor_output_path = ""

+# TODO: Temporary flag: If we are passing from export, ignore aot autograd for now
+ignore_aot_autograd = False


Let's move this and maybe _in_aot_compilation (my bad on that one) to virtualized.py, see: this pr

adding get_real_inputs to Virtualized.py

We replaced this with a from_export config, which is just used to tell us that this is from export and to skip over AOTAutograd. I plan to remove this flag within the next 2 weeks after fixing some other stuff on the export side. So is it ok to add it here for now?

torch/_inductor/freezing.py

torch/csrc/inductor/aot_inductor_model_container.h

torch/_inductor/codegen/wrapper.py

torch/_inductor/graph.py

test/inductor/test_inductor_freezing.py

facebook-github-bot · 2023-08-27T04:32:33Z

@muchulee8 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2023-08-27T18:11:52Z

@muchulee8 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2023-08-28T20:22:07Z

@muchulee8 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

ipiszy

Thanks @muchulee8 !

ipiszy · 2023-08-28T22:05:03Z

torch/_export/__init__.py

-    so_path = torch._inductor.aot_compile(ep.graph_module, list(all_args), options)
-    return so_path, ep
+    unlifted_module = ep.module()
+    unlifted_module.graph.set_codegen(torch.fx.CodeGen())  # type: ignore[attr-defined]


Wonder why is it necessary to do this? What's the CodeGen before this function call?

The CodeGen before this function call is a PytreeCodeGen, which I felt inductor didn't need to deal with at this time. It's in charge of making the input/output signature match the eager module, but I think with AOTInductor we just expect flat input/output so this is not really needed.

muchulee8 · 2023-08-29T17:36:39Z

@pytorchbot merge

pytorchmergebot · 2023-08-29T17:38:41Z

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team

Raised by workflow job

torch/csrc/inductor/aot_inductor_model_container.h

facebook-github-bot · 2023-08-29T19:18:04Z

@muchulee8 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

muchulee8 · 2023-08-29T21:07:52Z

@pytorchbot merge

pytorchmergebot · 2023-08-29T21:10:33Z

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

Summary: There's a deadlock in current storage's implementation if the size of tensor is too large. Use ctypes to do serialization. Test Plan: python benchmarks/dynamo/huggingface.py --bfloat16 --accuracy --inference --device cuda --export-aot-inductor --only MT5ForConditionalGeneration Reviewers: Subscribers: Tasks: Tags: Fixes #ISSUE_NUMBER Pull Request resolved: #108287 Approved by: https://github.com/desertfire, https://github.com/malfet

This reverts commit 43f28be. Reverted #108287 on behalf of https://github.com/desertfire due to Internal test failure from #107718. Revert this one first and then revert 107718. ([comment](#108287 (comment)))

facebook-github-bot · 2023-08-31T14:35:55Z

@pytorchbot revert -m="Diff reverted internally" -c="ghfirst"

This Pull Request has been reverted by a revert inside Meta. To re-land this change, please open another pull request, assign the same reviewers, fix the CI failures that caused the revert and make sure that the failing CI runs on the PR by applying the proper ciflow label (e.g., ciflow/trunk).)

pytorchmergebot · 2023-08-31T14:37:19Z

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot · 2023-08-31T14:37:25Z

Reverting PR 107718 failed

Reason: Command git -C /home/runner/work/pytorch/pytorch revert --no-edit c3239442a3dd1040b251ff33bef40589cba40e1c returned non-zero exit code 1

Auto-merging test/inductor/test_aot_inductor.py
CONFLICT (content): Merge conflict in test/inductor/test_aot_inductor.py
Auto-merging torch/_export/__init__.py
Auto-merging torch/_inductor/codegen/wrapper.py
Auto-merging torch/_inductor/compile_fx.py
Auto-merging torch/_inductor/config.py
Auto-merging torch/_inductor/graph.py
error: could not revert c3239442a3d... [AOTInductor] Include constants in AOTInductor .so file. (#107718)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git revert --continue".
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".

Details for Dev Infra team

Raised by workflow job

…7718)" This reverts commit c323944 due to internal test failures.

github-actions bot added module: inductor module: dynamo ciflow/inductor module: export labels Aug 22, 2023

muchulee8 force-pushed the mlee8/aot_weights_bc branch from b25625a to e75aa79 Compare August 22, 2023 19:11

muchulee8 requested a review from chenyang78 August 23, 2023 16:57

muchulee8 changed the title ~~[WIP] [AOTInductor] Include constants in AOTInductor .so file.~~ [AOTInductor] Include constants in AOTInductor .so file. Aug 23, 2023

muchulee8 requested review from angelayi, desertfire and eellison August 23, 2023 16:58

angelayi reviewed Aug 23, 2023

View reviewed changes

eellison reviewed Aug 23, 2023

View reviewed changes

muchulee8 force-pushed the mlee8/aot_weights_bc branch 2 times, most recently from bf547db to 04e2435 Compare August 23, 2023 23:05

muchulee8 requested review from angelayi and eellison August 23, 2023 23:50

muchulee8 force-pushed the mlee8/aot_weights_bc branch from 9479ecb to 3476544 Compare August 24, 2023 07:01

chenyang78 reviewed Aug 24, 2023

View reviewed changes

eellison reviewed Aug 24, 2023

View reviewed changes

muchulee8 force-pushed the mlee8/aot_weights_bc branch 2 times, most recently from 1bb9519 to e0badcf Compare August 27, 2023 03:48

muchulee8 requested a review from eellison August 28, 2023 20:28

ipiszy reviewed Aug 28, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 29, 2023

pytorchmergebot added the merging label Aug 29, 2023

pytorchmergebot removed the merging label Aug 29, 2023

angelayi added the release notes: export label Aug 29, 2023

eellison reviewed Aug 29, 2023

View reviewed changes

torch/csrc/inductor/aot_inductor_model_container.h Outdated Show resolved Hide resolved

torch/csrc/inductor/aot_inductor_model_container.h Show resolved Hide resolved

Add test for offset

b6e8801

muchulee8 requested a review from eellison August 29, 2023 19:31

eellison approved these changes Aug 29, 2023

View reviewed changes

pytorchmergebot added the merging label Aug 29, 2023

pytorchmergebot added Merged and removed merging labels Aug 29, 2023

pytorchmergebot closed this in c323944 Aug 29, 2023

desertfire mentioned this pull request Aug 31, 2023

Use ctypes to serialize raw content for tensors. #108287

Closed

desertfire added a commit that referenced this pull request Aug 31, 2023

Revert "[AOTInductor] Include constants in AOTInductor .so file. (#10…

193aed5

…7718)" This reverts commit c323944 due to internal test failures.

[AOTInductor] Include constants in AOTInductor .so file. #107718

[AOTInductor] Include constants in AOTInductor .so file. #107718

Uh oh!

Conversation

muchulee8 commented Aug 22, 2023 • edited by izaitsevfb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 22, 2023 • 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/107718

⏳ No Failures, 2 Pending

Uh oh!

linux-foundation-easycla bot commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Aug 22, 2023

Uh oh!

facebook-github-bot commented Aug 22, 2023

Uh oh!

facebook-github-bot commented Aug 22, 2023

Uh oh!

facebook-github-bot commented Aug 22, 2023

Uh oh!

angelayi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eellison left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 23, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eellison left a comment

Choose a reason for hiding this comment

Uh oh!

eellison Aug 24, 2023

Choose a reason for hiding this comment

Uh oh!

angelayi Aug 28, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

facebook-github-bot commented Aug 27, 2023

Uh oh!

facebook-github-bot commented Aug 27, 2023

Uh oh!

facebook-github-bot commented Aug 28, 2023

Uh oh!

ipiszy left a comment

Choose a reason for hiding this comment

Uh oh!

ipiszy Aug 28, 2023

Choose a reason for hiding this comment

Uh oh!

angelayi Aug 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

muchulee8 commented Aug 29, 2023

Uh oh!

pytorchmergebot commented Aug 29, 2023

Merge failed

Uh oh!

muchulee8 commented Aug 22, 2023 •

edited by izaitsevfb

Loading

pytorch-bot bot commented Aug 22, 2023 •

edited

Loading

linux-foundation-easycla bot commented Aug 22, 2023 •

edited

Loading

angelayi Aug 28, 2023 •

edited

Loading