native cpp extension with cmake #1006
Conversation
philei-tt
requested review from
ayerofieiev-tt,
jmalone-tt,
kevinwuTT and
dgomezTT
and removed request for
ayerofieiev-tt
docs/OpenRegistrationAPI.md
Outdated
| tt-smi 3.0.12 requires pre-commit==3.5.0, but you have pre-commit 3.0.4 which is incompatible. | ||
| ``` | ||
|
|
||
| ## Building cpp extension |
There was a problem hiding this comment.
Are these mutually exclusive options? If so I think we mention that in writing.
There was a problem hiding this comment.
I updated the instructions to separate the two paths here: 21fdf9c
docs/OpenRegistrationAPI.md
Outdated
| ```bash | ||
| git submodule sync | ||
| git submodule update --init --recursive | ||
| CMAKE_FLAGS="-DENABLE_SUBMODULE_TT_METAL_BUILD=ON" python3 setup.py develop |
There was a problem hiding this comment.
We should rewrite the steps when following the submodule path since we don't need to build metal separately, but we still need to create a new environment and build pytorch first. Or can we actually build pytorch as a third-party and metal at the same time with this path?
There was a problem hiding this comment.
I think we can build pytorch as third-party, but then we will have to try to figure out how to install python part of it from cmake. I think it's kind of logical that you don't need to build tt-metal, but I will add a comment. But if you won't build tt-metal, you still need to use ttnn from wheel, or we should figure out how to install python part of ttnn from built submodule 🤔
There was a problem hiding this comment.
I rewrote the instructions here: https://github.com/tenstorrent/pytorch2.0_ttnn/blob/21fdf9c5fd2d3c8bb5417e5296491cdeaf66d148/docs/OpenRegistrationAPI.md. It doesn't seem like pytorch needs to be rebuild for the submodule path? Granted, I've only tested the simple unit test, and I haven't encountered any errors yet.
There was a problem hiding this comment.
It works, but it's hard to be 100% sure that it won't break in the future. I think it will work as long as we guarantee that submodule version == ttnn wheel version. Ideally we should install ttnn as python package from submodule, but we can do that later in the future
There was a problem hiding this comment.
Can you add a change to this PR in workflows/update-ttnn-wheel.yaml to update the metal submodule commit to match the updated wheel release? This should automatically keep our wheel and metal submodule in sync.
torch_ttnn/cpp_extension/ttnn_cpp_extension/src/utils/vector_utils.cpp
Outdated
Show resolved
Hide resolved
…anges around that
Co-authored-by: Kevin Wu <[email protected]>
|
|
||
| ### Build torchvision (Optional) | ||
| Optionally, you can build torchvision, which might be required for some computer-vision models: | ||
| ```bash |
There was a problem hiding this comment.
I think we need to remove the indents, because it looks like this in the preview:
Ticket
Original PR: #806
Problem description
This branch is based on https://github.com/tenstorrent/pytorch2.0_ttnn/tree/kw/cpp_extension
What's changed
On top of points mentioned in #806
Ideally we won't be recompiling torch, but compile ttnn in a compatible with torch variant (gcc + c++17 + some other flags used in torch), but that will be the next PR