Skip to content

[TorchFix] Add torch.solve as a removed function #4705

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

Merged
merged 1 commit into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions tools/torchfix/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,19 @@ To enable them, use standard flake8 configuration options for the plugin mode or

If you encounter a bug or some other problem with TorchFix, please file an issue on
https://github.com/pytorch/test-infra/issues, mentioning [TorchFix] in the title.


## Rules

### TOR001 Use of removed function

#### torch.solve
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a doc/page that calls out this function as deprecated? If so, what do you think of linking to it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it's removed from the docs.
The YAML in the PR has links to deprecate and remove PRs, I plan later to use these links to generate some docs.


This function was deprecated since PyTorch version 1.9 and is now removed.

`torch.solve` is deprecated in favor of `torch.linalg.solve`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like we still mention torch.solve in our docs. Could you update that ref too please?

https://pytorch.org/docs/stable/complex_numbers.html
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good find! I'll update that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

`torch.linalg.solve` has its arguments reversed and does not return the LU factorization.

To get the LU factorization see `torch.lu`, which can be used with `torch.lu_solve` or `torch.lu_unpack`.

`X = torch.solve(B, A).solution` should be replaced with `X = torch.linalg.solve(A, B)`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import torch
A = torch.randn(3, 3)
b = torch.randn(3)
torch.solve(A, b).solution
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4:1 TOR001 Use of removed function torch.solve: https://github.com/pytorch/test-infra/tree/main/tools/torchfix#torchsolve
5 changes: 5 additions & 0 deletions tools/torchfix/torchfix/deprecated_symbols.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
- name: torch.solve
deprecate_pr: https://github.com/pytorch/pytorch/pull/57741
remove_pr: https://github.com/pytorch/pytorch/pull/70986
reference: https://github.com/pytorch/test-infra/tree/main/tools/torchfix#torchsolve

- name: torch.qr
deprecate_pr: https://github.com/pytorch/pytorch/pull/57745
remove_pr:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ def visit_Call(self, node):
message = f"Use of removed function {qualified_name}"
replacement = self._call_replacement(node, qualified_name)

reference = self.deprecated_config[qualified_name].get("reference")
if reference is not None:
message = f"{message}: {reference}"

self.violations.append(
LintViolation(
error_code=error_code,
Expand Down