Fix for mul(compressed, wrapped scalar)
#91239
Conversation
nikitaved
added
module: sparse
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/91239
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit b049b8f: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
nikitaved
force-pushed
the
nikitaved/csr_scalar_mul_fix
branch
from
743dfcb
to
238207d
Compare
There was a problem hiding this comment.
I have a few nits but overall looks good! Thanks, @nikitaved!
test/test_sparse_csr.py
Outdated
| if dtype == torch.result_type(sparse, scalar) and not enable_hybrid: | ||
| res_in_dense = sparse.to_dense().mul_(scalar) | ||
| res_in = sparse.mul_(scalar) | ||
| self.assertEqual(res_in.to_dense(), res_in_dense) |
There was a problem hiding this comment.
Nit as above:
| self.assertEqual(res_in.to_dense(), res_in_dense) | |
| self.assertEqual(res_in, res_in_dense) |
There was a problem hiding this comment.
Is it tested? :)
There was a problem hiding this comment.
yes, it originates from #88749
| if (t_.layout() == kStrided && src_.is_sparse_csr()) { | ||
| return mul_out_sparse_csr(t_.sparse_mask(src_), src_, r); | ||
| } | ||
| TORCH_CHECK(r.is_sparse_csr(), "Expected result Tensor to be of format CSR"); |
There was a problem hiding this comment.
Just a note for a possible follow-up: generalizing this function to other sparse compressed layouts looks like a straightforward task. It would remove one of the blockers in test_gradcheck_sparse_csc_input of test/test_autograd.py.
|
Dispatcher is acting weirdly here. There is a |
…taved/csr_scalar_mul_fix
…taved/csr_scalar_mul_fix
|
@pearu, could you please unblock by greening your review? |
|
@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 |
Fixes pytorch#90819. The path with `Scalar` should have been picked up by the dispatcher, but still the path with a 0-dim wrapped scalar was broken. Pull Request resolved: pytorch#91239 Approved by: https://github.com/pearu, https://github.com/cpuhrsch
Fixes #90819.
The path with
Scalarshould have been picked up by the dispatcher, but still the path with a 0-dim wrapped scalar was broken.cc @pearu @cpuhrsch @amjames @bhosmer