Avoid differing results in linalg.(tensor_)solve
#154983
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/154983
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New FailuresAs of commit 80295cd with merge base 81dbeb0 ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
linalg.(tensor_)solve
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
the mps failures are real. @malfet is there anything that needs changing in the mps path? |
Merge failedReason: 3 jobs have failed, first few of them are: trunk / macos-py3-arm64 / test (mps, 1, 1, macos-m2-15), trunk / macos-py3-arm64 / test (mps, 1, 1, macos-m1-14), trunk / macos-py3-arm64 / test (mps, 1, 1, macos-m1-13) Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
|
Successfully rebased |
e49864e to
1ab6e77
Compare
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
|
Successfully rebased |
1ab6e77 to
dcd1070
Compare
|
Looks like the MPS implementation either differs or has a workaround to account for the potential difference in the current implementation. Is there anyone that knows enough about the MPS backend to clarify or investigate this? |
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
Remove an optimization potentially using a transposed matrix as input for `linalg_lu_factor_ex_out`. Depending on whether the input memory layout is contiguous or not this may lead to slightly different results which may cause larger differences in subsequent steps ultimately leading to test failures in e.g. `test_vmapvjp_linalg_tensorsolve_cpu_float32` & `test_vmapvjpvjp_linalg_tensorsolve_cpu_float32`. The intended optimization no longer applies after 59bc76f so this code can be removed too resolving the accuracy issues observed in those tests. Fixes #151440
|
Successfully rebased |
dcd1070 to
80295cd
Compare
Remove an optimization potentially using a transposed matrix as input for
linalg_lu_factor_ex_out.Depending on whether the input memory layout is contiguous or not this may lead to slightly different results which may cause larger differences in subsequent steps ultimately leading to test failures in e.g.
test_vmapvjp_linalg_tensorsolve_cpu_float32&test_vmapvjpvjp_linalg_tensorsolve_cpu_float32.The intended optimization no longer applies after 59bc76f so this code can be removed too resolving the accuracy issues observed in those tests.
With this change the code path used for the "regular" and "vmap" cases are identical: A batched tensor is iterated over in the batch dimension in lu_solve and lu_factor
Prior to this it might not be the case as either tensor would/could have been non-contiguous leading to using a transposed tensor for the LU factorization instead.
So the (CPU) results should now be identical.
Fixes #151440
Fixes #114868