Feature/colpiv qr in nalgebra lapack #1551
Conversation
nalgebra-lapack/src/colpiv_qr.rs
Outdated
| /// routine for column pivoting QR decomposition using level 3 BLAS, | ||
| /// see https://www.netlib.org/lapack/lug/node42.html | ||
| /// or https://www.intel.com/content/www/us/en/docs/onemkl/developer-reference-c/2023-0/geqp3.html | ||
| fn xgeqp3( |
There was a problem hiding this comment.
I think that all functions of ColPivQrScalar must be unsafe because underlying lapack functions do not check the lengths of slices.
There was a problem hiding this comment.
You're right. Frankly, I think all the lapack trait functions in this crate must be unsafe. That is something I already have on my agenda for later, but I will definitely update the stuff inside my current PR.
nalgebra-lapack/src/colpiv_qr.rs
Outdated
| xormqr = $xormqr:path $(,)? | ||
| ) => { | ||
| impl ColPivQrReal for $type { | ||
| fn xormqr( |
nalgebra-lapack/src/colpiv_qr.rs
Outdated
| // without pivoting. I'm not 100% sure that we can't abstract over real and | ||
| // complex behavior in the scalar trait, but I'll keep it like this for now. | ||
| #[allow(missing_docs)] | ||
| pub unsafe trait ColPivQrReal: ColPivQrScalar + QRReal { |
There was a problem hiding this comment.
I think that unsafe is not required here for a whole trait as merely implementing this trait does not make any safe code that does not use any unsafe methods of this trait itself unsafe.
Otherwise safety-related changes LGTM.
There was a problem hiding this comment.
Yeah, I went a bit overboard, but my thought was that I'd like to basically prevent people from implementing those traits themselves. I think what I'll actually do is make those traits sealed.
nalgebra-lapack/src/colpiv_qr.rs
Outdated
| /// Utility trait to add a thin abstraction layer over lapack functionality for | ||
| /// column pivoted QR decomposition. | ||
| #[allow(missing_docs)] | ||
| pub unsafe trait ColPivQrScalar: ComplexField + QRScalar { |
2 participants
This PR adds column-pivoted QR decomposition to
nalgebra-lapack. This is a pretty useful feature, because while the "normal"nalgebracrate also has column-pivoted QR, it will only solve square linear systems. Thenalgebra-lapackimplementation can be used for both square and overdetermined linear systems, which should be a useful addition.UPDATE:
lapack-srccrate, which fixes Undefined Behavior and Segfaults When Linking MKL in nalgebra-lapack #1552.