sparse-linear-algebra icon indicating copy to clipboard operation
sparse-linear-algebra copied to clipboard

Incorrect bounds checking.

Open GregorySchwartz opened this issue 7 years ago • 5 comments

Prelude Data.Sparse.Common> x = (fromListSV 4 [(2,3)] :: SpVector Double)
Prelude Data.Sparse.Common> y = (fromListSV 4 [(0,3)] :: SpVector Double)
Prelude Data.Sparse.Common> fromRowsL [x, y]
*** Exception: insertRowSM : incompatible dimensions (2,4)
CallStack (from HasCallStack):
  error, called at src/Data/Sparse/Common.hs:79:17 in sparse-linear-algebra-0.2.9.8-GSb8IwES68r5Fh6hC5eCZ9:Data.Sparse.Common

Here, x and y are rows with the same dimension, so from the name I would think that fromRowsL [x, y] would result in a 2x4 matrix. Further evidence:

Prelude Data.Sparse.Common> m
SM ( 2 rows, 4 columns ) , 8 NZ ( density 100.000 % ) [(0,[(0,0.0),(1,0.0),(2,3.0),(3,0.0)]),(1,[(0,3.0),(1,0.0),(2,0.0),(3,0.0)])]
Prelude Data.Sparse.Common> fromRowsL $ toRowsL m
*** Exception: insertRowSM : incompatible dimensions (2,4)
CallStack (from HasCallStack):
  error, called at src/Data/Sparse/Common.hs:79:17 in sparse-linear-algebra-0.2.9.8-GSb8IwES68r5Fh6hC5eCZ9:Data.Sparse.Common

toColsL and fromColsL both work it seems.

GregorySchwartz avatar Jan 04 '18 21:01 GregorySchwartz

Uhh, this one is bad. Thanks again @GregorySchwartz , will fix this soon.

BTW What are you using the library for, if I may? I'm in the middle of a large, performance-oriented rewrite and I'd love to hear your use case to prioritize features (e.g. data size/density, crucial operations etc.)

ocramz avatar Jan 04 '18 21:01 ocramz

My use case is research, especially with big data. Crucial operations commonly used are the most basic matrix and vector operations (addition, subtraction, multiplication, matrix multiplication, indexing, slicing, filtering, index based filtering, etc.). The linear algebra that is most important in my entire field is PCA, eigenvector and eigenvalue decomposition, and SVD. I can't emphasize enough that those three are used pretty much everywhere, and the easier they are to use the better (i.e. helper functions such that one can go MAT -> (EIGENVECTORS, EIGENVALUES) are invaluable). Lastly, I have an interest in moving these operations to the GPU and I noticed that you plan to use accelerate, so I was very interested in that.

GregorySchwartz avatar Jan 04 '18 22:01 GregorySchwartz

Is this fixed? I'm still getting the same error with my first example above in version 0.3.1.

GregorySchwartz avatar Aug 13 '19 15:08 GregorySchwartz

@GregorySchwartz no, apparently it's not fixed. Could you provide a minimal repro and/or perhaps send a patch? Thanks

ocramz avatar Aug 13 '19 15:08 ocramz

Latest master does not have this problem -- the most recent hackage release is behind the fix. Can you upload the current version to hackage?

GregorySchwartz avatar Aug 13 '19 18:08 GregorySchwartz