Skip to content
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

nan propagation in matrix multiplication #340

Closed
ivirshup opened this issue Apr 27, 2020 · 4 comments · Fixed by #469
Closed

nan propagation in matrix multiplication #340

ivirshup opened this issue Apr 27, 2020 · 4 comments · Fixed by #469
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@ivirshup
Copy link
Contributor

ivirshup commented Apr 27, 2020

Describe the bug

Matrix multiplication does not propagate nan like numpy does.

To Reproduce

import numpy as np
import sparse

A = np.eye(4)
A[2, 2] = np.nan

np.eye(4) @ A
# array([[ 1.,  0., nan,  0.],
#        [ 0.,  1., nan,  0.],
#        [ 0.,  0., nan,  0.],
#        [ 0.,  0., nan,  1.]])

(sparse.eye(4) @ sparse.COO(A)).todense()
# array([[ 1.,  0.,  0.,  0.],
#        [ 0.,  1.,  0.,  0.],
#        [ 0.,  0., nan,  0.],
#        [ 0.,  0.,  0.,  1.]])

Expected behavior

I would expect the same values from numpy and sparse. However, I see that nan breaks the expected sparsity pattern, which could make efficient implementation difficult. Maybe a warning or error would be appropriate?

System

  • OS and version: macOS 10.15.3
  • sparse version (sparse.__version__) '0.9.1'
  • NumPy version (np.__version__) '1.18.3'
  • Numba version (numba.__version__) '0.49.0'

Additional context

@ivirshup ivirshup added the bug Indicates an unexpected problem or unintended behavior label Apr 27, 2020
@hameerabbasi
Copy link
Collaborator

Yes, this is because we (perhaps incorrectly) assume that 0 * x == 0 in matmul, and nowhere else.

@hameerabbasi
Copy link
Collaborator

Is there a use-case for this behaviour?

@ivirshup
Copy link
Contributor Author

I'm doing outer joins, which I've ended up implementing with matrix multiplication since that works fairly consistently across array types. This was an inconsistency I came across. I've already worked around it, just thought it would be good for this behavior to be defined since I wasn't expecting this.

FWIW, Julia sparse arrays do the same thing.
using LinearAlgebra, SparseArrays

S = sparse(1.0I, 4, 4)
S[3, 3] = NaN
sparse(I, 4, 4) * S
# 4×4 SparseMatrixCSC{Float64,Int64} with 4 stored entries:
#   [1, 1]  =  1.0
#   [2, 2]  =  1.0
#   [3, 3]  =  NaN
#   [4, 4]  =  1.0

@hameerabbasi
Copy link
Collaborator

A short-term solution here would be to have a warning (like you said), (PRs welcome). If we want a long-term solution, we're looking for #365.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants