-
Notifications
You must be signed in to change notification settings - Fork 607
Revert adding pytorch-triton as a build requirement #2592
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
base: main
Are you sure you want to change the base?
Conversation
…rch-triton as it is a placeeholder in pyPI Signed-off-by: tdophung <tdophung@nvidia.com>
for more information, see https://pre-commit.ci
Greptile SummaryThis PR removes Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant pip
participant setup.py
participant pytorch.py
participant PyTorch
User->>pip: pip install transformer_engine_torch
pip->>setup.py: Execute setup
setup.py->>pytorch.py: Call install_requirements()
alt Before this PR
pytorch.py-->>setup.py: Returns list with pytorch-triton
setup.py->>pip: Install pytorch-triton (placeholder on PyPI)
Note over pip: Would fail: pytorch-triton on PyPI<br/>is a placeholder
end
alt After this PR
pytorch.py-->>setup.py: Returns list without pytorch-triton
setup.py->>pip: Install dependencies (no pytorch-triton)
Note over pip: Success: Users already have<br/>pytorch-triton from torch install
end
User->>PyTorch: Already has pytorch-triton<br/>from torch installation
Note over User,PyTorch: torch.compile() can use<br/>pre-installed pytorch-triton
|
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.
Additional Comments (1)
-
build_tools/pytorch.py, line 16-26 (link)style: docstring is now outdated since
pytorch-tritonis no longer in the requirements list
1 file reviewed, 1 comment
|
Could you change the docstring too @tdophung? Since it now incorrectly mentions pytorch-triton. |
Signed-off-by: tdophung <tdophung@nvidia.com>
Description
Since pytorch-triton is a placeholder in PyPI, it should not be auto fetched and install in TE build. Users that uses torch should already have pytorch-triton installed when they install torch whl.
WE should also not revert this to requires 'triton' either because it is currently not compatible with torch.compile
Fixes # (issue)
Type of change
Changes
remove pytorch-triton from list of requirements in TE PyTorch build
Checklist: