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

Disable LTC by default until upstream revert relands #1303

Merged
merged 3 commits into from
Aug 29, 2022

Conversation

powderluv
Copy link
Collaborator

Tracked with the WIP #1292

Disable LTC in setup.py temporarily until upstream is fixed.
Copy link
Member

@henrytwo henrytwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the issues are due to PyTorch constantly reverting and unreverting that change which changed some function signatures, right?

When @antoniojkim reverts part of his changed (or PyTorch reverts again...), I assume this should be fixed?

Is there anything we can do to improve the stability here? It seems that we kinda just have to deal with this due to usually the latest PyTorch master

@henrytwo
Copy link
Member

Oh wait minor comment, perhaps we should have a comment to link back to the issue, in case people are confused about why it's disabled?

This way we also know what code to "rollback" when reenabling

@ashay
Copy link
Collaborator

ashay commented Aug 29, 2022

We face a similar issue internally (even before the current revert/re-revert pattern), so we pin the PyTorch version in requirements.txt and bump it the most recent PyTorch version when we update Torch-MLIR. Is that (i.e. updating PyTorch on demand) something that we would be open to doing here as well?

@powderluv powderluv merged commit c0630da into main Aug 29, 2022
@powderluv
Copy link
Collaborator Author

@henrytwo yes added a comment.

@ashay yes we can pin & roll. Ideally we can automate the roll so we don't fall too behind (or even have an informational top of main build so we know if there is a breakage).

@silvasean
Copy link
Contributor

We face a similar issue internally (even before the current revert/re-revert pattern), so we pin the PyTorch version in requirements.txt and bump it the most recent PyTorch version when we update Torch-MLIR. Is that (i.e. updating PyTorch on demand) something that we would be open to doing here as well?

Yes, I think that moving to a model where we update PyTorch on demand would be better here. We already do that for LLVM/MLIR, and now that LTC gives us so much more exposure to PyTorch churn I think it would benefit from the same kind of approach.

@powderluv powderluv deleted the powderluv-temp-disable-ltc branch August 29, 2022 23:33
qedawkins pushed a commit to nod-ai/torch-mlir that referenced this pull request Oct 3, 2022
qedawkins pushed a commit to nod-ai/torch-mlir that referenced this pull request Oct 3, 2022
The modification to use the builder based interface to generate Krnl loop is completed (llvm#1250, llvm#1283, llvm#1285, llvm#1292, llvm#1293, llvm#1303, llvm#1308, llvm#1314, llvm#1316, llvm#1317, llvm#1326, llvm#1403), and BuildKrnlLoop is no longer needed.

Signed-off-by: Whitney Tsang [email protected]
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.

4 participants