[Split Build] Use single command to build both wheels #126590
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/126590
Note: Links to docs will display an error until the docs builds have been completed. ❌ 22 New Failures, 1 Unrelated FailureAs of commit 998b193 with merge base 2b72e2a ( NEW FAILURES - The following jobs have failed:
UNSTABLE - The following job failed but was likely due to flakiness present on trunk and has been marked as unstable:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
ghstack-source-id: 3627b4513e48d9231ef28ccb811c2fd264a42a21 Pull Request resolved: #126590
ghstack-source-id: 6cfbe22b4d83307ea39877723eb8b86f7f13ad27 Pull Request resolved: #126590
ghstack-source-id: b673d085f6078646573940456713659bc541ed8a Pull Request resolved: #126590
ghstack-source-id: 818ba4d5df3eace02bd90bd932ba2f81e4faf3ac Pull Request resolved: #126590
ghstack-source-id: 101ca8c5a187a9a364ae149ffca21d031b602992 Pull Request resolved: #126590
ghstack-source-id: 754885f2f6bfe7304ee587e2500d3b32a777eb97 Pull Request resolved: #126590
ghstack-source-id: 9bc0681e005c8d357f8f27afc9cae4675ddb2efb Pull Request resolved: #126590
ghstack-source-id: 2f12726f8c6b93f5e5cdd73670df4cc0324d911f Pull Request resolved: #126590
ghstack-source-id: a3dbd1096f4087b9a2df48da006e25b5eca1667a Pull Request resolved: #126590
ghstack-source-id: 6e7f1bc1ede8e2a73f22eebac4983dc117e3f031 Pull Request resolved: #126590
ghstack-source-id: c906ffb04fce5912d9381bfed20f0f738e7a4858 Pull Request resolved: #126590
Adds a `SPLIT_BUILD` option to build both the libtorch wheel and pytorch wheels at the same time. This option was added as a convenience and right now only supports "clean" and "install" [ghstack-poisoned]
| @@ -230,20 +235,8 @@ def _get_package_path(package_name): | |||
| return None | |||
|
|
|||
|
|
|||
| BUILD_LIBTORCH_WHL = os.getenv("BUILD_LIBTORCH_WHL", "0") == "1" | |||
There was a problem hiding this comment.
These two PRs are wholly unrelated. The only reason they're together in a stack is to not have a merge conflict. I can just remove this PR and CI should still pass if you want
| or setup_cmd == "sdist" | ||
| or setup_cmd == "develop" | ||
| ): | ||
| raise RuntimeError( |
There was a problem hiding this comment.
I'm a bit confusing here, if this raises a RunTimeError, then the rest of the if statement would be skipped, wouldn't it?
There was a problem hiding this comment.
I guess this is unsupported use case, looking at the Error text.
| global BUILD_PYTHON_ONLY | ||
| global PACKAGE_NAME | ||
|
|
||
| BUILD_LIBTORCH_WHL = os.getenv("BUILD_LIBTORCH_WHL", "0") == "1" |
There was a problem hiding this comment.
Nit: Is there a reason to bring the initialization BUILD_LIBTORCH_WHL = os.getenv("BUILD_LIBTORCH_WHL", "0") == "1" and BUILD_PYTHON_ONLY = os.getenv("BUILD_PYTHON_ONLY", "0") == "1" here instead of where they are defined above? It's better to defined and init a variable in the same place if possible IMO
| _main() | ||
|
|
||
|
|
||
| def _main(): |
There was a problem hiding this comment.
I would love to call this function something else if possible _main doesn't feel like a good name, may be _build? The context here is that I was asking myself why there are 2 main functions, then I realized that one is _main
There was a problem hiding this comment.
+1 for this, using _build sounds like a good idea
| BUILD_PYTHON_ONLY = True | ||
| sys.argv[1] = "clean" | ||
| PACKAGE_NAME = final_package_name | ||
| _main() |
There was a problem hiding this comment.
Will this work same way as ?:
BUILD_LIBTORCH_WHL=1 python setup.py install
python setup.py clean
BUILD_PYTHON_ONLY=1 python setup.py install
The code above is calling _main() 3 times. While code here is only 2 times, combining clean step with BUILD_PYTHON_ONLY=1.
| ) | ||
|
|
||
| final_package_name = PACKAGE_NAME | ||
| PACKAGE_NAME = "libtorch" |
There was a problem hiding this comment.
What are the final 2 package names ? Where we will store them in https://download.pytorch.org/whl/nightly/ - we want to make sure there do not come to conflict with current torch packages.
To better explain this point, please take a look nightly workflow results, here: https://github.com/pytorch/pytorch/actions/runs/9209764850?pr=126699
looks like the packages with split option and regular packages are almost the same:
Looking at the package contents, shows the same structure:
METADATA is same as well
Adds a `SPLIT_BUILD` option to build both the libtorch wheel and pytorch wheels at the same time. This option was added as a convenience and right now only supports "clean" and "install" [ghstack-poisoned]
Stack from ghstack (oldest at bottom):
Adds a
SPLIT_BUILDoption to build both the libtorch wheel and pytorch wheels at the same time. This option was added as a convenience and right now only supports "clean" and "install"