refactor: move dynamo/TorchXLA bridge to pytorch/xla repo by shunting314 · Pull Request #4476 · pytorch/xla

shunting314 · 2023-01-19T01:14:12Z

Check the pytorch side PR for details: pytorch/pytorch#92601 .

The pytorch one needs to wait for this one to be merged first.

shunting314 · 2023-01-19T01:46:13Z

@JackCaoG should I be worried about the CI break?

for the lint error, it looks like xla/ repo wants 2 spaces indent while pytorch/ repo usually uses 4. I can run some quick 'sed' command to format the code. Or let me know if there is already some script in the repo that can do the job.
the build error seems to be caused by some unrelated network error.

JackCaoG · 2023-01-19T01:56:25Z

I just restarted the CI, let's see how it goes. I think you also want to update the test under test/dynamo since the bridge is under a new path.

shunting314 · 2023-01-19T02:00:57Z

Looks like the exsting tests under test/dynamo does not refer to the bridge directly. They specify the backend by string name which is not affected by the refactoring.

wconstab · 2023-01-19T05:10:28Z

should I review the code in this PR as if it is just a code move from pytorch/pytorch or is there also some significant change to the code?

shunting314 · 2023-01-19T05:17:14Z

@wconstab there is no significant change. Just some code movements and some necessary renames.

JackCaoG · 2023-01-20T01:21:00Z

Thanks Shutning, I will try to take a look tmr.

JackCaoG

Mostly LGTM, thanks Shunting!

JackCaoG

Let's fix the linter then we can merge this one. Thanks @shunting314 !

shunting314 · 2023-01-24T18:45:55Z

@JackCaoG any good way for me to fix the lint errors?
I already fixed a few but what I'm doing right now is too inefficient:
1, check the CI lint error log
2, the CI lint error log only report the first couple of lint errors, so I go ahead to fix them manually
3, go back to 1

How do you guys fix lint errors? Any more efficient ways? (hopefully there is some script to do that locally?)

JackCaoG · 2023-01-24T18:46:45Z

You can follow https://github.com/pytorch/xla/blob/master/CONTRIBUTING.md#before-submiting-a-pull-request

JackCaoG · 2023-01-24T20:21:22Z

failure is unrealted, merging

This is a follow up from the previous PR: #88449 , to move the dynamo/TorchXLA bridge from pytorch repo to xla repo. Overall the dynamo/TorchXLA integration has the following four layers of code - pybind layer: This is the bottom layer containing various pybind APIs as the foundation. This part resident in xla repo - bridge layer: build upon the pybind layer to implement the trace once functionality. This layer and it's corresponding unit test are in pytorch repro previously. This PR (and the corresponding xla pr pytorch/xla#4476 ) moves them to the xla repo. - dynamo backend registration: this a thin layer registers 4 dynamo backends (training/inference/trace_once/trace_everytime). It remains in pytorch repo. - benchmark script: the torchbench.py script in dynamo is adapted so it can be used in dynamo/TorchXLA integration. This one remains in pytorch repo. We think the new code organization is cleaner. I'll wait for the xla PR in first before trying to merge this one. Tests 1. run the unit tests moved to the xla repo 2. Test for inference: `GPU_NUM_DEVICES=1 python benchmarks/dynamo/torchbench.py --randomize-input --performance --trace-on-xla --backend=torchxla_trace_once --only resnet18` 3. Test for training: `GPU_NUM_DEVICES=1 python benchmarks/dynamo/torchbench.py --randomize-input --performance --trace-on-xla --training --backend=aot_torchxla_trace_once --only resnet18 --collect-outputs` Pull Request resolved: #92601 Approved by: https://github.com/wconstab

) This is a follow up from the previous PR: pytorch#88449 , to move the dynamo/TorchXLA bridge from pytorch repo to xla repo. Overall the dynamo/TorchXLA integration has the following four layers of code - pybind layer: This is the bottom layer containing various pybind APIs as the foundation. This part resident in xla repo - bridge layer: build upon the pybind layer to implement the trace once functionality. This layer and it's corresponding unit test are in pytorch repro previously. This PR (and the corresponding xla pr pytorch/xla#4476 ) moves them to the xla repo. - dynamo backend registration: this a thin layer registers 4 dynamo backends (training/inference/trace_once/trace_everytime). It remains in pytorch repo. - benchmark script: the torchbench.py script in dynamo is adapted so it can be used in dynamo/TorchXLA integration. This one remains in pytorch repo. We think the new code organization is cleaner. I'll wait for the xla PR in first before trying to merge this one. Tests 1. run the unit tests moved to the xla repo 2. Test for inference: `GPU_NUM_DEVICES=1 python benchmarks/dynamo/torchbench.py --randomize-input --performance --trace-on-xla --backend=torchxla_trace_once --only resnet18` 3. Test for training: `GPU_NUM_DEVICES=1 python benchmarks/dynamo/torchbench.py --randomize-input --performance --trace-on-xla --training --backend=aot_torchxla_trace_once --only resnet18 --collect-outputs` Pull Request resolved: pytorch#92601 Approved by: https://github.com/wconstab

shunting314 requested review from JackCaoG and wconstab January 19, 2023 01:15

JackCaoG added the dynamo label Jan 19, 2023

shunting314 mentioned this pull request Jan 19, 2023

refactor: move dynamo/TorchXLA bridge to pytorch/xla repo pytorch/pytorch#92601

Closed

shunting314 force-pushed the dynamo-xla-refactor branch from d971fa3 to cb857fb Compare January 19, 2023 01:58