Remove deprecated torch.solve#70986
Conversation
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. |
🔗 Helpful links
❌ 1 New FailuresAs of commit 155b7f5 (more details on the Dr. CI page): Expand to see more
🕵️ 1 new failure recognized by patternsThe following CI failures do not appear to be due to upstream breakages
|
lezcano
left a comment
There was a problem hiding this comment.
I won't cry when this one's gone either. Now, I left a few comments regarding the MAGMA / LAPACK functions that we're removing.
| } | ||
|
|
||
| template<> void lapackSolve<float>(int n, int nrhs, float *a, int lda, int *ipiv, float *b, int ldb, int *info) { | ||
| sgesv_(&n, &nrhs, a, &lda, ipiv, b, &ldb, info); |
There was a problem hiding this comment.
Why don't we want to keep these kernels? Shouldn't they be more efficient than lu + lu_solve?
There was a problem hiding this comment.
No, they are not more efficient. gesv = getrf + getrs but done in an external library. We should leave only the code that is used.
| template<> | ||
| void magmaSolve<double>( | ||
| magma_int_t n, magma_int_t nrhs, double* dA, magma_int_t ldda, | ||
| magma_int_t* ipiv, double* dB, magma_int_t lddb, magma_int_t* info) { | ||
| MagmaStreamSyncGuard guard; | ||
| magma_dgesv_gpu(n, nrhs, dA, ldda, ipiv, dB, lddb, info); | ||
| AT_CUDA_CHECK(cudaGetLastError()); | ||
| } |
albanD
left a comment
There was a problem hiding this comment.
Thanks for the PR Ivan.
Do you think it would be possible to add to torch/__init__.py something like:
# This function was depreacted and removed.
# This nice error message can be removed in version 1.3+
def solve(input, A, out=None):
raise RuntimeError("This function was deprecated since version 1.9 and is now removed. Please use the `torch.linalg.solve` function instead")To make sure that users get a nice error message for a couple versions?
|
Sure, I will add that. |
|
Alright, I added a In [1]: import torch
In [2]: a = torch.randn(3, 3)
In [3]: b = torch.randn(3, 3)
In [4]: torch.solve(b, a)
---------------------------------------------------------------------------
RuntimeError Traceback (most recent call last)
<ipython-input-4-6f9b9974b5b1> in <module>
----> 1 torch.solve(b, a)
RuntimeError: This function was deprecated since version 1.9 and is now removed. Please use the `torch.linalg.solve` function instead.
In [5]: b.solve(a)
---------------------------------------------------------------------------
RuntimeError Traceback (most recent call last)
<ipython-input-5-fd669f2275c3> in <module>
----> 1 b.solve(a)
RuntimeError: This function was deprecated since version 1.9 and is now removed. Please use the `torch.linalg.solve` function instead. |
|
If you ever manage to land this, remind me to remove in #74046 the backward functions for |
| # triangular_solve goes through specific backend dispatch (CPU/CUDA) and hit auto-generated device check first | ||
| generic_backend_dispatch_err_str = "Expected b and A to be on the same device" | ||
| specific_backend_dispatch_err_str = "Expected all tensors to be on the same device" | ||
| with self.assertRaisesRegex(RuntimeError, generic_backend_dispatch_err_str): |
There was a problem hiding this comment.
nit: could you leave a small test here (or in test_torch.py) that the error you added is working as expected?
|
I added a test checking that the error is raised as expected and resolved accumulated merge conflicts. |
|
@pytorchbot merge this please |
|
Hey @IvanYashchuk. |
Summary: The time has come to remove deprecated linear algebra related functions. This PR removes `torch.solve`. cc jianyuh nikitaved pearu mruberry walterddr IvanYashchuk xwang233 Lezcano Pull Request resolved: #70986 Approved by: https://github.com/Lezcano, https://github.com/albanD Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/890bdf13e17adcd0ede255afbf5abf29b7d8d6ee Reviewed By: malfet Differential Revision: D36299674 Pulled By: malfet fbshipit-source-id: fd01095c72787103d16c442595f67f5020f393fb
The time has come to remove deprecated linear algebra related functions. This PR removes `torch.solve`. cc @jianyuh @nikitaved @pearu @mruberry @walterddr @IvanYashchuk @xwang233 @lezcano Pull Request resolved: pytorch#70986 Approved by: https://github.com/Lezcano, https://github.com/albanD
The time has come to remove deprecated linear algebra related functions. This PR removes
torch.solve.cc @jianyuh @nikitaved @pearu @mruberry @walterddr @IvanYashchuk @xwang233 @lezcano