Remove deprecated torch.lstsq #70980
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
IvanYashchuk
added
module: linear algebra
IvanYashchuk
requested review from
albanD,
ezyang,
nikitaved and
soulitzer
as code owners
CI Flow Status⚛️ CI FlowRuleset - Version:
You can add a comment to the PR and tag @pytorchbot with the following commands: # ciflow rerun, "ciflow/default" will always be added automatically
@pytorchbot ciflow rerun
# ciflow rerun with additional labels "-l <ciflow/label_name>", which is equivalent to adding these labels manually and trigger the rerun
@pytorchbot ciflow rerun -l ciflow/scheduled -l ciflow/slowFor more information, please take a look at the CI Flow Wiki. |
Contributor
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit aaf7113 (more details on the Dr. CI page):
🕵️ 20 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
3 tasks
dagitses
added
the
triaged
lezcano
approved these changes
Jan 11, 2022
Collaborator
lezcano
left a comment
lezcano
left a comment
There was a problem hiding this comment.
Just left a small comment out of curiosity, but it LGTM
|
|
||
| // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ legacy_lstsq ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| std::tuple<Tensor, Tensor> legacy_lstsq_cuda(const Tensor &B, const Tensor &A) { |
Collaborator
There was a problem hiding this comment.
This seems a bit odd. Did we reimplement all the calling code for magmaGels within linalg_lstsq_cuda? I guess we did because of the TORCH_WARN_ONCE that's here, but that seems so odd.
lstsq is a mess, it could do with a reasonable clean-up.
Collaborator
Author
There was a problem hiding this comment.
Porting of TH version of lstsq to ATen was done independently from implementing torch.linalg.lstsq.
Collaborator
There was a problem hiding this comment.
Fantastic to get rid of this code then (same applies to all the other PRs, but to this one in particular).
Collaborator
Author
|
@pytorchbot ciflow rerun |
|
Hi @IvanYashchuk , Is this #71222 a known issue? |
Collaborator
|
@de-gozaru , I have had some issues with the determinism of linalg.lstsq while working on its forward/backward ADs. If my memory serves me well, it was the case with only wide or tall matrices. |
kit1980
approved these changes
Sep 22, 2022
Collaborator
Author
|
@pytorchbot merge -g |
Collaborator
|
@pytorchbot successfully started a merge job. Check the current status here. |
Collaborator
Merge failedReason: This PR is too stale; the last push date was more than 3 days ago. Please rebase and try again. Details for Dev Infra teamRaised by workflow job |
Collaborator
Author
|
@pytorchbot merge |
Collaborator
Merge failedReason: The following mandatory check(s) are pending/not yet run (Rule
Dig deeper by viewing the pending checks on hud Details for Dev Infra teamRaised by workflow job |
Collaborator
Author
|
@pytorchbot merge -g |
Collaborator
Merge failedReason: The following mandatory check(s) are pending/not yet run (Rule
Dig deeper by viewing the pending checks on hud Details for Dev Infra teamRaised by workflow job |
Contributor
|
@kit1980 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
|
@pytorchbot merge -g |
Collaborator
|
@pytorchbot successfully started a merge job. Check the current status here. |
Collaborator
Merge failedReason: Command Details for Dev Infra teamRaised by workflow job |
Collaborator
Author
|
I resolved the merge conflict. |
Contributor
|
@kit1980 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
|
@pytorchbot merge -g |
Collaborator
|
@pytorchbot successfully started a merge job. Check the current status here. |
Collaborator
Merge failedReason: 1 additional jobs have failed, first few of them are: trunk Details for Dev Infra teamRaised by workflow job |
Contributor
|
@pytorchbot merge -f "Unrelated ROCm issue, tests passed previously" |
Collaborator
|
@pytorchbot successfully started a merge job. Check the current status here. |
Contributor
|
Hey @IvanYashchuk. |
mehtanirav
pushed a commit
that referenced
this pull request
Oct 4, 2022
The time has come to remove deprecated linear algebra related functions. This PR removes `torch.lstsq`. There's a note in `tools/codegen/gen.py` about `lstsq` schema in `native_function.yaml` that I will not remove: https://github.com/pytorch/pytorch/blob/87139d8532c99ff5dbeef1b97948d71793aa7851/tools/codegen/gen.py#L734-L770 cc @jianyuh @nikitaved @pearu @mruberry @walterddr @IvanYashchuk @xwang233 @lezcano Pull Request resolved: #70980 Approved by: https://github.com/lezcano, https://github.com/kit1980
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The time has come to remove deprecated linear algebra related functions. This PR removes
torch.lstsq.There's a note in
tools/codegen/gen.pyaboutlstsqschema innative_function.yamlthat I will not remove:pytorch/tools/codegen/gen.py
Lines 734 to 770 in 87139d8
cc @jianyuh @nikitaved @pearu @mruberry @walterddr @IvanYashchuk @xwang233 @lezcano