Support torchbind in OSS proxy executor #149747

yushangdi · 2025-03-21T17:37:44Z

Summary:
Implement torchbind support in OSSProxyExecutor.

Exactly the same as the implementation in FbProxyExecutor.

D69693697 - fbProxyExecutor
D69887230 - fbProxyExecutor but for torchbind method

Other changes:

When generating the schema of the CallTrochBind HOP, the arg name of the torchbind object arg should be the same as the torchbind method's torchbind object arg (instead of obj).
In AOTIModelPackageLoader, we extract everything in data/constants to tmp_dir/data/aot_inductor/<model>/ folder, so the torchbind objs exist in the same folder as the rest of the files (e.g. cpp, so). This is to be consistent of how files are packaged internally

Test Plan:

buck run fbcode//mode/dev-nosan //caffe2/test/inductor:torchbind -- -r torchbind_aoti

buck run fbcode//mode/dev-nosan //caffe2/test/inductor:torchbind -- -r aot_compile

Differential Revision: D69500038

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

pytorch-bot · 2025-03-21T17:37:48Z

🔗 Helpful Links

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

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

❌ 5 New Failures, 5 Unrelated Failures

As of commit 4ee3e67 with merge base 01cb351 ():

NEW FAILURES - The following jobs have failed:

linux-binary-libtorch-release / libtorch-cpu-shared-with-deps-release-build / build (gh)
Process completed with exit code 1.
linux-binary-manywheel / manywheel-py3_9-cuda11_8-build / build (gh)
Process completed with exit code 1.
linux-binary-manywheel / manywheel-py3_9-cuda12_6-build / build (gh)
Process completed with exit code 1.
linux-binary-manywheel / manywheel-py3_9-cuda12_8-build / build (gh)
Process completed with exit code 1.
pull / linux-jammy-py3.9-gcc11 / test (backwards_compat, 1, 1, lf.ephemeral.linux.2xlarge) (gh)
test_modules_can_be_imported

FLAKY - The following job failed but was likely due to flakiness present on trunk:

trunk / win-vs2022-cpu-py3 / test (default, 2, 3, lf.windows.4xlarge.nonephemeral) (gh) (similar failure)
Process completed with exit code 1.

BROKEN TRUNK - The following jobs failed but was present on the merge base:

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

pull / linux-focal-py3.13-clang10 / test (dynamo_wrapped, 3, 3, lf.ephemeral.linux.2xlarge) (gh) (trunk failure)
test_nestedtensor.py::TestNestedTensorSubclassCPU::test_composite_op_in_inference_mode_cpu_float32
pull / linux-focal-py3.9-clang10 / test (dynamo_wrapped, 3, 3, lf.ephemeral.linux.2xlarge) (gh) (trunk failure)
test_nestedtensor.py::TestNestedTensorSubclassCPU::test_composite_op_in_inference_mode_cpu_float32

UNSTABLE - The following jobs are marked as unstable, possibly due to flakiness on trunk:

inductor / linux-jammy-cpu-py3.9-gcc11-inductor / test (dynamic_cpu_inductor_torchbench, 1, 2, linux.8xlarge.amx) (gh) (#149979)
detectron2_fcos_r_50_fpn
pull / cuda12.4-py3.10-gcc9-sm75 / test (pr_time_benchmarks, 1, 1, linux.g4dn.metal.nvidia.gpu) (gh) (#149370)
REGRESSION: benchmark ('aotdispatcher_partitioner_cpu2', 'compile_time_instruction_count') failed, actual result 1747971565 is 1.86% higher than expected 1716000000 ±+1.50% if this is an expected regression, please update the expected results.

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

facebook-github-bot · 2025-03-21T17:37:54Z

This pull request was exported from Phabricator. Differential Revision: D69500038

Skylion007 · 2025-03-21T18:35:42Z

torch/csrc/inductor/aoti_package/model_package_loader.cpp

C10 has a shim for this already. https://github.com/pytorch/pytorch/blob/main/caffe2/utils/string_utils.h

Or better yet: https://github.com/pytorch/pytorch/blob/main/c10/util/string_view.h

Skylion007 · 2025-03-21T18:40:24Z

torch/csrc/inductor/aoti_torch/oss_proxy_executor.h

Arg name should be moved into the initializer std::move

facebook-github-bot · 2025-03-21T19:58:16Z

This pull request was exported from Phabricator. Differential Revision: D69500038

Summary: Implement torchbind support in OSSProxyExecutor. Exactly the same as the implementation in FbProxyExecutor. D69693697 - fbProxyExecutor D69887230 - fbProxyExecutor but for torchbind method Other changes: - When generating the schema of the CallTrochBind HOP, the arg name of the torchbind object arg should be the same as the torchbind method's torchbind object arg (instead of `obj`). - In `AOTIModelPackageLoader`, we extract everything in `data/constants` to `tmp_dir/data/aot_inductor/<model>/` folder, so the torchbind objs exist in the same folder as the rest of the files (e.g. cpp, so). This is to be consistent of how files are packaged internally Note: Seems like there'll be issues with using`filesystem` header in linux: pytorch#137209 So here I use string manipulation instead of `filesystem::path` Test Plan: ``` buck run fbcode//mode/dev-nosan //caffe2/test/inductor:torchbind -- -r torchbind_aoti buck run fbcode//mode/dev-nosan //caffe2/test/inductor:torchbind -- -r aot_compile buck2 build --flagfile fbsource//arvr/mode/win/dev fbsource//arvr/libraries/audio/AudioSDK/Research/Source/BSI/UnitTests:meridian_bsi_tests buck2 build --flagfile fbsource//arvr/mode/embedded/linux/clang-aarch64-release fbsource//arvr/third-party/signalarity/latest-release/src/shaker/ha:signalarity_realtime_libtorch_pipeline_ ``` Differential Revision: D69500038

facebook-github-bot · 2025-03-21T20:31:08Z

This pull request was exported from Phabricator. Differential Revision: D69500038

zou3519

_higher_order_ops change LGTM, someone else who knows the AOTI code should take a look at that

facebook-github-bot · 2025-03-25T20:03:49Z

This pull request was exported from Phabricator. Differential Revision: D69500038

Summary: Pull Request resolved: pytorch#149747 Implement torchbind support in OSSProxyExecutor. Exactly the same as the implementation in FbProxyExecutor. D69693697 - fbProxyExecutor D69887230 - fbProxyExecutor but for torchbind method Other changes: - When generating the schema of the CallTrochBind HOP, the arg name of the torchbind object arg should be the same as the torchbind method's torchbind object arg (instead of `obj`). - In `AOTIModelPackageLoader`, we extract everything in `data/constants` to `tmp_dir/data/aot_inductor/<model>/` folder, so the torchbind objs exist in the same folder as the rest of the files (e.g. cpp, so). This is to be consistent of how files are packaged internally Note: Seems like there'll be issues with using`filesystem` header in linux: pytorch#137209 So here I use string manipulation instead of `filesystem::path` Test Plan: ``` buck run fbcode//mode/dev-nosan //caffe2/test/inductor:torchbind -- -r torchbind_aoti buck run fbcode//mode/dev-nosan //caffe2/test/inductor:torchbind -- -r aot_compile buck2 build --flagfile fbsource//arvr/mode/win/dev fbsource//arvr/libraries/audio/AudioSDK/Research/Source/BSI/UnitTests:meridian_bsi_tests buck2 build --flagfile fbsource//arvr/mode/embedded/linux/clang-aarch64-release fbsource//arvr/third-party/signalarity/latest-release/src/shaker/ha:signalarity_realtime_libtorch_pipeline_ ``` Differential Revision: D69500038

desertfire · 2025-03-27T13:06:53Z

torch/csrc/inductor/aoti_torch/oss_proxy_executor.cpp

-    OSSOpKernel& op_kernel) {
-  auto& stack = op_kernel.stack_;
-  auto& dynamic_args = op_kernel.dynamic_args_;
+    OSSOpKernel* op_kernel,


Curious why this refernce to pointer change?

Never mind. I see your created child classes later.

desertfire · 2025-03-27T13:18:50Z

torch/csrc/inductor/aoti_package/model_package_loader.cpp

+      } else { // startsWith(filename_str, const_directory)
+        // Extract constants to the same directory as the rest of the files
+        // to be consistent with internal implementation
+        size_t lastSlash = filename_str.find_last_of("/\\");


You can just use k_separator here.

You can just use k_separator here.

changed to use k_separator now

Summary: Implement torchbind support in OSSProxyExecutor. Exactly the same as the implementation in FbProxyExecutor. D69693697 - fbProxyExecutor D69887230 - fbProxyExecutor but for torchbind method D70746626 - Support None output type Other changes: - When generating the schema of the CallTrochBind HOP, the arg name of the torchbind object arg should be the same as the torchbind method's torchbind object arg (instead of `obj`). - In `AOTIModelPackageLoader`, we extract everything in `data/constants` to `tmp_dir/data/aot_inductor/<model>/` folder, so the torchbind objs exist in the same folder as the rest of the files (e.g. cpp, so). This is to be consistent of how files are packaged internally (more details in internal Diff summary). Note on using `filesystem`: Seems like there'll be [issues](pytorch#137209) with using`filesystem` header in linux, so here I use string manipulation instead of `filesystem::path`. Test Plan: ``` buck run fbcode//mode/dev-nosan //caffe2/test/inductor:torchbind -- -r torchbind_aoti buck run fbcode//mode/dev-nosan //caffe2/test/inductor:torchbind -- -r aot_compile buck2 build --flagfile fbsource//arvr/mode/win/dev fbsource//arvr/libraries/audio/AudioSDK/Research/Source/BSI/UnitTests:meridian_bsi_tests buck2 build --flagfile fbsource//arvr/mode/embedded/linux/clang-aarch64-release fbsource//arvr/third-party/signalarity/latest-release/src/shaker/ha:signalarity_realtime_libtorch_pipeline_ ``` Reviewed By: desertfire Differential Revision: D69500038

facebook-github-bot · 2025-03-27T17:55:32Z

This pull request was exported from Phabricator. Differential Revision: D69500038

facebook-github-bot · 2025-03-27T21:39:42Z

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

pytorchmergebot · 2025-03-27T21:41:31Z

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

facebook-github-bot · 2025-03-28T02:46:25Z

@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 · 2025-03-28T02:47:55Z

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

This reverts commit aa70d62. Reverted #149747 on behalf of https://github.com/facebook-github-bot due to Diff reverted internally ([comment](#149747 (comment)))

pytorchmergebot · 2025-03-28T02:48:05Z

@yushangdi your PR has been successfully reverted.

facebook-github-bot · 2025-03-28T03:00:16Z

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

pytorchmergebot · 2025-03-28T03:02:03Z

Merge started

Your change will be merged while ignoring the following 0 checks:

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 · 2025-03-28T03:02:20Z

Merge failed

Reason: 1 jobs have failed, first few of them are: Meta Internal-Only Changes Check

Details for Dev Infra team

Raised by workflow job

yangw-dev · 2025-03-28T04:05:09Z

@pytorchbot merge -i

yushangdi · 2025-03-28T20:04:18Z

relanding in #150196

Summary: Implement torchbind support in OSSProxyExecutor. Exactly the same as the implementation in FbProxyExecutor. D69693697 - fbProxyExecutor D69887230 - fbProxyExecutor but for torchbind method Other changes: - When generating the schema of the CallTrochBind HOP, the arg name of the torchbind object arg should be the same as the torchbind method's torchbind object arg (instead of `obj`). - In `AOTIModelPackageLoader`, we extract everything in `data/constants` to `tmp_dir/data/aot_inductor/<model>/` folder, so the torchbind objs exist in the same folder as the rest of the files (e.g. cpp, so). This is to be consistent of how files are packaged internally Test Plan: ``` buck run fbcode//mode/dev-nosan //caffe2/test/inductor:torchbind -- -r torchbind_aoti buck run fbcode//mode/dev-nosan //caffe2/test/inductor:torchbind -- -r aot_compile ``` Differential Revision: D69500038 Pull Request resolved: pytorch#149747 Approved by: https://github.com/desertfire

This reverts commit aa70d62. Reverted pytorch#149747 on behalf of https://github.com/facebook-github-bot due to Diff reverted internally ([comment](pytorch#149747 (comment)))

yushangdi requested a review from zou3519 as a code owner March 21, 2025 17:37

pytorch-bot bot added ciflow/inductor module: inductor labels Mar 21, 2025

facebook-github-bot added the fb-exported label Mar 21, 2025

yushangdi requested review from SherlockNoMad, desertfire and zhxchen17 March 21, 2025 17:38

yushangdi added the release notes: export label Mar 21, 2025

Skylion007 reviewed Mar 21, 2025

View reviewed changes

yushangdi force-pushed the export-D69500038 branch from 94bce25 to fcf70f0 Compare March 21, 2025 19:58

yushangdi force-pushed the export-D69500038 branch from fcf70f0 to 1707a22 Compare March 21, 2025 20:30

zou3519 reviewed Mar 25, 2025

View reviewed changes

yushangdi force-pushed the export-D69500038 branch from 1707a22 to 1645ea2 Compare March 25, 2025 19:56

yushangdi force-pushed the export-D69500038 branch from 1645ea2 to 332dc10 Compare March 25, 2025 20:03

desertfire approved these changes Mar 27, 2025

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Mar 27, 2025

yushangdi force-pushed the export-D69500038 branch from 332dc10 to 4ee3e67 Compare March 27, 2025 17:55

pytorchmergebot added the merging label Mar 27, 2025

pytorchmergebot closed this in aa70d62 Mar 28, 2025

pytorchmergebot added Merged and removed merging labels Mar 28, 2025

pytorchmergebot added Reverted ci-no-td Do not run TD on this PR labels Mar 28, 2025

pytorchmergebot reopened this Mar 28, 2025

pytorchmergebot added the merging label Mar 28, 2025

pytorchmergebot removed the merging label Mar 28, 2025

pytorchmergebot added merging and removed merging labels Mar 28, 2025

pytorch deleted a comment from pytorchmergebot Mar 28, 2025

yushangdi closed this Mar 28, 2025

Support torchbind in OSS proxy executor #149747

Support torchbind in OSS proxy executor #149747

Uh oh!

Conversation

yushangdi commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Mar 21, 2025 • 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/149747

❌ 5 New Failures, 5 Unrelated Failures

Uh oh!

facebook-github-bot commented Mar 21, 2025

Uh oh!

Skylion007 Mar 21, 2025 • 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!

Skylion007 Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

Skylion007 Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Mar 21, 2025

Uh oh!

facebook-github-bot commented Mar 21, 2025

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Mar 25, 2025

Uh oh!

desertfire Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

desertfire Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

desertfire Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

yushangdi Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Mar 27, 2025

Uh oh!

facebook-github-bot commented Mar 27, 2025

Uh oh!

pytorchmergebot commented Mar 27, 2025

Merge started

Uh oh!

facebook-github-bot commented Mar 28, 2025

Uh oh!

pytorchmergebot commented Mar 28, 2025

Uh oh!

pytorchmergebot commented Mar 28, 2025

Uh oh!

facebook-github-bot commented Mar 28, 2025

Uh oh!

pytorchmergebot commented Mar 28, 2025

Merge started

Uh oh!

pytorchmergebot commented Mar 28, 2025

Merge failed

Uh oh!

yangw-dev commented Mar 28, 2025

Uh oh!

yushangdi commented Mar 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

yushangdi commented Mar 21, 2025 •

edited

Loading

pytorch-bot bot commented Mar 21, 2025 •

edited

Loading

Skylion007 Mar 21, 2025 •

edited

Loading