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

Incorrect bounds checking. #42

Open
GregorySchwartz opened this issue Jan 4, 2018 · 5 comments
Open

Incorrect bounds checking. #42

GregorySchwartz opened this issue Jan 4, 2018 · 5 comments
Assignees
Labels

Comments

@GregorySchwartz
Copy link
Contributor

GregorySchwartz commented Jan 4, 2018

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.

@ocramz ocramz added the bug label Jan 4, 2018
@ocramz ocramz self-assigned this Jan 4, 2018
@ocramz
Copy link
Owner

ocramz commented Jan 4, 2018

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.)

@GregorySchwartz
Copy link
Contributor Author

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.

@Jaxan Jaxan mentioned this issue Mar 12, 2018
3 tasks
@ocramz ocramz added this to Selected for development in sparse-linear-algebra Apr 27, 2018
@ocramz ocramz closed this as completed in f9453f0 Apr 27, 2018
sparse-linear-algebra automation moved this from Selected for development to Done + Code review Apr 27, 2018
@GregorySchwartz
Copy link
Contributor Author

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

@ocramz ocramz reopened this Aug 13, 2019
@ocramz
Copy link
Owner

ocramz commented Aug 13, 2019

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

@GregorySchwartz
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
sparse-linear-algebra
  
Done + Code review
Development

No branches or pull requests

2 participants