Skip to content
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.

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

[Release] Fix binary name for downstream compatibility #3752

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

sjain-stanford
Copy link
Member

@sjain-stanford sjain-stanford commented Oct 1, 2024

As of Sep 14, the torch-mlir binary wheels got renamed to torch-mlir-core from torch-mlir: image

This was an unintended side-effect of the recent change of TORCH_MLIR_ENABLE_ONLY_MLIR_PYTHON_BINDINGS=True (#3711) which skips setting NAME = "torch-mlir" in setup.py.

To avoid having multiple downstreams fix their pip deps, this change allows using the same torch-mlir name for binaries, and reserves a separate torch-mlir-ext name for the (less popular) binaries with extensions enabled.

@sjain-stanford sjain-stanford changed the title fix binary name for downstream compatibility [Release] Fix binary name for downstream compatibility Oct 1, 2024
@saienduri
Copy link
Collaborator

saienduri commented Oct 1, 2024

Fine by me, as torch-mlir-core seems to be the main thing that we support and test right now so makes sense to change to torch-mlir. Can you also change all the other occurrences of these package names in the repo to torch-mlir and torch-mlir-ext so we have uniform naming convention? Let's also wait for Stella's opinion (she's probably the one that named them initially lol)

@sjain-stanford
Copy link
Member Author

sjain-stanford commented Oct 2, 2024

Can you also change all the other occurrences of these package names in the repo to torch-mlir and torch-mlir-ext so we have uniform naming convention?

@saienduri I have done this now, but just for the record, until now we were really calling the release builds with PT extensions (TM_PACKAGES=torch-mlir) here - which ran build_torch_mlir.

After this change, we will actually be building the "core" (non-PT-extension) version. It is unlikely this breaks anything downstream for y'all but if it does, we could simply switch this line in the GHA workflow to go back:

- TM_PYTHON_VERSIONS=cp310-cp310 TM_PACKAGES=torch-mlir ./build_tools/python_deploy/build_linux_packages.sh
+ TM_PYTHON_VERSIONS=cp310-cp310 TM_PACKAGES=torch-mlir-ext ./build_tools/python_deploy/build_linux_packages.sh

I'm going to go ahead and land this. 🤞🏻

@sjain-stanford sjain-stanford merged commit f8e4a9a into llvm:main Oct 2, 2024
3 checks passed
@sjain-stanford sjain-stanford deleted the sambhav/pip_name branch October 2, 2024 18:52
sjain-stanford added a commit to cruise-automation/mlir-tcp that referenced this pull request Oct 3, 2024
This was long overdue, but we had torch-mlir releases
[broken](https://github.com/llvm/torch-mlir-release/actions/runs/10838435405)
for a while in September, and the new ones were released under a
different name (fixed by llvm/torch-mlir#3752).

Also, since torch-mlir is no longer released with PT extensions, I had
to explicitly specify the cpu nightly link in requirements, otherwise it
pulls the gpu torch with a bunch of unnecessary nvidia/cuda deps.

Bump `torch` from `2.4.0.dev20240604+cpu` to `2.6.0.dev20241002+cpu`.
Bump `torch-mlir` from `20240714.152` to `20241002.240`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants