-
Notifications
You must be signed in to change notification settings - Fork 611
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
Bump torch-mlir to 98e08023bbf71e00ab81e980eac9f7c96f1f24b4 #18388
Conversation
c7d14fe
to
5394fb4
Compare
5394fb4
to
e9b036f
Compare
78fb290
to
8212a81
Compare
Signed-off-by: jinchen62 <[email protected]>
8212a81
to
e26faa3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
llvm/torch-mlir@fa39d91 included here seems to have broken some downstream usage of the IREE Python bindings: https://github.com/iree-org/iree-turbine/actions/runs/10617974632/job/29432221240?pr=112#step:5:204
We may be missing some test coverage in this repo.
Can someone verify / repro, then decide if we should revert or fix forward?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can repro with just IREE:
- Build from source: https://iree.dev/building-from-source/getting-started/#building-with-cmake
- Setup venv and install some deps:
python -m venv .venv .venv\Scripts\activate.bat D:\dev\projects\iree-build\.env.bat python -m pip install onnx torch sympy
- Try running
>>> from iree.compiler.extras.fx_importer import ContextCache Traceback (most recent call last): File "<stdin>", line 1, in <module> File "D:\dev\projects\iree-build\compiler\bindings\python\iree\compiler\extras\fx_importer.py", line 38, in <module> import torch File "D:\dev\scratch\iree\.venv\Lib\site-packages\torch\__init__.py", line 2120, in <module> from torch._higher_order_ops import cond File "D:\dev\scratch\iree\.venv\Lib\site-packages\torch\_higher_order_ops\__init__.py", line 1, in <module> from .cond import cond File "D:\dev\scratch\iree\.venv\Lib\site-packages\torch\_higher_order_ops\cond.py", line 5, in <module> import torch._subclasses.functional_tensor File "D:\dev\scratch\iree\.venv\Lib\site-packages\torch\_subclasses\functional_tensor.py", line 42, in <module> class FunctionalTensor(torch.Tensor): File "D:\dev\scratch\iree\.venv\Lib\site-packages\torch\_subclasses\functional_tensor.py", line 258, in FunctionalTensor cpu = _conversion_method_template(device=torch.device("cpu")) D:\dev\scratch\iree\.venv\Lib\site-packages\torch\_subclasses\functional_tensor.py:258: UserWarning: Failed to initialize NumPy: _ARRAY_API not found (Triggered internally at C:\actions-runner\_work\pytorch\pytorch\builder\windows\pytorch\torch\csrc\utils\tensor_numpy.cpp:84.) cpu = _conversion_method_template(device=torch.device("cpu")) Traceback (most recent call last): File "<stdin>", line 1, in <module> File "D:\dev\projects\iree-build\compiler\bindings\python\iree\compiler\extras\fx_importer.py", line 138, in <module> from .._mlir_libs._torchMlir import get_int64_max, get_int64_min ModuleNotFoundError: No module named 'iree.compiler._mlir_libs._torchMlir'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't repro from running https://github.com/iree-org/iree/blob/main/compiler/bindings/python/test/extras/fx_importer_test.py though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. https://github.com/iree-org/iree/blob/main/compiler/bindings/python/test/extras/fx_importer_test.py does catch this, if torch
is installed. Should get that test really running, together with #16624. Maybe a new workflow similar to https://github.com/iree-org/iree/blob/main/.github/workflows/pkgci_test_tensorflow_cpu.yml focused on torch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding / enabling test coverage that would have caught this in #18420
I'd say we revert this change. |
@MaheshRavishankar @ScottTodd So should I revert this PR or revert that torch-mlir commit? |
Added a comment to the upstream PR. Still seeing if we can work around it locally in the meantime. |
But while we wait on movement upstream (or contribute a patch ourselves), I vote we revert this since it breaks some usage of IREE's Python APIs that iree-turbine depends on. |
…18405) Reverts #18388 llvm/torch-mlir@fa39d91 included in bump commit breaks some downstream usage of the IREE Python bindings that iree-turbine depends on. Revert it while we wait on movement upstream.
…#18388) Signed-off-by: jinchen62 <[email protected]>
…ree-org#18388)" This reverts commit 462612d.
…#18388) Signed-off-by: jinchen62 <[email protected]>
…ree-org#18405) Reverts iree-org#18388 llvm/torch-mlir@fa39d91 included in bump commit breaks some downstream usage of the IREE Python bindings that iree-turbine depends on. Revert it while we wait on movement upstream.
No description provided.