Support rectangular solve and qr.solve compilation - #82
Conversation
- Use dgesv when A is known square - Infer solve() output dims from ncol(A) - Add tests for least-squares and codegen calls
- Add r2f handler for qr.solve() with tests - Fix Fortran subscript emission for hoisted temporaries - Validate compiled names and preserve mixed-case C bindings
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
==========================================
+ Coverage 92.08% 92.30% +0.21%
==========================================
Files 22 22
Lines 4472 4560 +88
==========================================
+ Hits 4118 4209 +91
+ Misses 354 351 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 484c509e7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| hoist$emit(glue( | ||
| "call dgesv({blas_int(m)}, {blas_int(nrhs)}, {A_work@name}, {blas_int(m)}, {ipiv@name}, {out_name}, {blas_int(m)}, {info@name})" | ||
| )) |
There was a problem hiding this comment.
Because qr.solve() now dispatches to lapack_solve(), any input whose dimensions are proven square takes the dgesv LU branch (this call), which ignores QR rank‑deficiency handling and tol. For singular or near‑singular square matrices (e.g., collinear columns), qr.solve() is expected to use QR to produce a least‑squares solution or signal rank issues; here it will return whatever dgesv leaves in B (and info is never checked), so results can be incorrect or undefined in exactly the cases users call qr.solve for.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed by skipping the LU/dgesv fast path for qr.solve. In lapack_solve() the square-matrix branch now guards on !identical(context, "qr.solve"), so qr.solve always routes through the QR path (with rank/tol handling) even when inputs are square.
There was a problem hiding this comment.
Also added tests to cover the qr.solve path via QR (including square inputs), so we exercise the rank/tol handling and keep the LU fast path out of qr.solve.
- Route qr.solve through dgelsy (QR pivoting) - Accept scalar tol and thread to rcond - Add tests for solver routing and name suggestions
1 participant
Summary