search

yig / PySPQR

Python wrapper for the sparse QR decomposition in SuiteSparseQR.
Creative Commons Zero v1.0 Universal
34 stars 28 forks source link

Expose SuiteSparseQR_C_backslash to Python as qr_solve. #2

Closed Technologicat closed 7 years ago

Technologicat commented 7 years ago

Hi,

Thanks for the wrapper, almost exactly what I was looking for. :)

As for the "almost" part, here is a quick addition to PySPQR that allows solving A x = b in the least-squares sense. Using SuiteSparseQR_C_backslash for this is usually much faster than first explicitly constructing Q via SuiteSparseQR_C_QR (although this approach is now also documented in the docstrings).

I also took the liberty to remove trailing whitespace, and added/expanded some docstrings.

I've tested this with both the small test in main() and an approximately 4500x1500 matrix in a project I'm working on, and it works. Solve time for the 4500x1500 is under 3 seconds on an i7 (whereas first decomposing with qr() takes about a minute).

I'm not 100% sure if I did all the low-level stuff (memory handling / data type conversion) correctly - comments are welcome.

yig commented 7 years ago

Thanks! I merged the changes. I made a change to fix a memory error. I added a .copy() to the use of cffi_asarray, because that returns a view.

Technologicat commented 7 years ago

Ok. Thanks for the fix!

Just a minute ago, I updated the README to reflect that the solvers are now also wrapped (and some minor edits to the examples), do you want to pull that too?

yig commented 7 years ago

I think I pulled those. It seems that pull requests automatically update to include all changes in your fork.

Technologicat commented 7 years ago

Correct. However, there were some last-minute edits that didn't make it into the pull :)

Also, there was a potentially confusing copy'n'paste typo in one my added docstrings in spqr.py; that is now also fixed in my fork.

yig commented 7 years ago

Got it. I pulled those changes now, too.

Technologicat commented 7 years ago

Thanks for your patience!

And... if you want, there's one more new last-minute edit in the README.md in my fork: I just now noticed that Markdown supports syntax highlighting. It's now enabled for the examples.

Now I think I probably won't make more edits to this any time soon :)