Skip to content

[BUG] Incorrect behavior of sparse matrix-matrix multiplication #6219

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

Closed
kose-y opened this issue Apr 3, 2018 · 1 comment
Closed

[BUG] Incorrect behavior of sparse matrix-matrix multiplication #6219

kose-y opened this issue Apr 3, 2018 · 1 comment
Assignees

Comments

@kose-y
Copy link

kose-y commented Apr 3, 2018

This PyTorch bug was introduced by #4707.
Unlike claimed there, indices should not only be unique, but also be sorted in coalesced matrix for correct behavior of matrix multiplication. This raises a major issue when performing matrix multiplication with coalesced then transposed matrix.

The following is tested on 2e156f3, but any version after 5e72d7a will have the same behavior.

import torch
idx = torch.LongTensor([[0,1,2], [2,1,0]])
val = torch.ones(3)
D = torch.sparse.FloatTensor(idx,val,torch.Size([3,3]))
Dc = D.coalesce()
x = torch.ones((3,1))
torch.mm(D.t(), x)
 1
 1
 1
[torch.FloatTensor of size (3,1)]
torch.mm(Dc.t(), x)
 3
 0
 0
[torch.FloatTensor of size (3,1)]

I will create a pull request regarding this and #6171 later today.

@weiyangfb
Copy link
Contributor

weiyangfb commented Aug 13, 2018

This issue is caused by the following:

  • t() because it swaps the indices of a sparse tensor without setting coalesce=false
  • calling coalesce() sets coalesce=true
  • mm(Sparse, Dense) doesn't really sort the sparse tensor - the call to coalesce() doesn't do anything.

A simple fix will be to set coalesce=false at t()

This is related to the invariants of transpose(): #4707, maybe should change coalesce slight to get around it.

After chatted to @zou3519, we can define a sparse tensor with coalesced=true when:

  1. its elements are unique and
  2. the indices are in sorted order

And set coalesce=false whenever the conditions are not held, e.g. t(). This should solve the issue.

zdevito pushed a commit to zdevito/ATen that referenced this issue Aug 15, 2018
…iants (#10496)

Summary:
- fixes pytorch/pytorch#6219
- removed invariants at pytorch/pytorch#4707
- assume a sparse tensor with coalesced=true when:
1. its elements are unique and
2. the indices are in sorted order
Pull Request resolved: pytorch/pytorch#10496

Differential Revision: D9311214

Pulled By: weiyangfb

fbshipit-source-id: 167fa5a8e9e5f9c800db02f728a1194029f7e4f3
goodlux pushed a commit to goodlux/pytorch that referenced this issue Aug 15, 2018
…iants (pytorch#10496)

Summary:
- fixes pytorch#6219
- removed invariants at pytorch#4707
- assume a sparse tensor with coalesced=true when:
1. its elements are unique and
2. the indices are in sorted order
Pull Request resolved: pytorch#10496

Differential Revision: D9311214

Pulled By: weiyangfb

fbshipit-source-id: 167fa5a8e9e5f9c800db02f728a1194029f7e4f3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants