-
Notifications
You must be signed in to change notification settings - Fork 198
Pivoting QR decomposition #1045
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
base: master
Are you sure you want to change the base?
Conversation
|
I think this PR is almost ready to be reviewed. I have the specs left to write but this should pretty quick as it mainly is a small modification of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for QR factorization with column pivoting to the linear algebra library. The implementation leverages the LAPACK geqp3 routine and extends the existing qr interface to support pivot tracking.
- Adds pivoting QR factorization via
geqp3LAPACK routine - Extends
qr()andqr_space()interfaces to support column pivoting with pivot output - Includes comprehensive test coverage for tall, wide, and rank-deficient matrices
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/linalg/test_linalg_pivoting_qr.fypp | Comprehensive test suite for pivoting QR factorization across different matrix types and configurations |
| test/linalg/CMakeLists.txt | Adds pivoting QR test to build system |
| src/stdlib_linalg_qr.fypp | Implements pivoting QR factorization routines and workspace calculation |
| src/stdlib_linalg_lapack.fypp | Adds LAPACK geqp3 interface for QR with column pivoting |
| src/stdlib_linalg.fypp | Extends public interface with pivoting QR subroutines |
| src/lapack/stdlib_linalg_lapack_aux.fypp | Adds error handler for geqp3 LAPACK routine |
| example/linalg/example_pivoting_qr_space.f90 | Example demonstrating pivoting QR with pre-allocated storage |
| example/linalg/example_pivoting_qr.f90 | Basic example demonstrating pivoting QR factorization |
| example/linalg/CMakeLists.txt | Adds pivoting QR examples to build system |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1045 +/- ##
=========================================
Coverage ? 25.12%
=========================================
Files ? 570
Lines ? 234201
Branches ? 41276
=========================================
Hits ? 58854
Misses ? 175347
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Modulo the issue with the |
jvdp1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @loiseaujc . Overall LGTM. I just have some minor suggestions.
perazz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this implementation @loiseaujc, great work.
I stand by @jvdp1's comments, and I added a few edits/typos.
Overall, looks good to me with the above.
Co-authored-by: Federico Perini <federico.perini@gmail.com>
Co-authored-by: Federico Perini <federico.perini@gmail.com>
Co-authored-by: Federico Perini <federico.perini@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Federico Perini <federico.perini@gmail.com>
|
For the build |
Following #930, this PR extends the
qrinterface to enable the computation of the QR factorization with column pivoting. It is based on thexGEQP3driver fromlapack.Proposed interfaces
call qr(a, q, r, pivots [, overwrite_a, storage, err])where
ais the matrix to be factorized,qthe orthonormal basis forcolspan(a),rthe upper triangular matrix andpivotsan integer array with indices of the pivoted columns.Progress
Ping: @perazz, @jvdp1, @jalvesz