Skip to content

Query SGELQF workspace in SGELSS, as D/C/ZGELSS do - #1428

Merged
langou merged 1 commit into
Reference-LAPACK:masterfrom
kyungminlee:fix-sgelss-workspace
Sep 26, 2026
Merged

langou merged 1 commit into
Reference-LAPACK:masterfrom
kyungminlee:fix-sgelss-workspace

Conversation

@kyungminlee

Copy link
Copy Markdown
Contributor

SGELSS still computes the SGELQF term of its Path 2a workspace as M*ILAENV(1, 'SGELQF', ...), which repeats the formula inside the reference SGELQF instead of asking SGELQF. Commit 9e96fbe (bug 0065, LAPACK 3.4.0) replaced the ILAENV workspace formulas in DGELSS, CGELSS and ZGELSS with workspace queries, and they have asked ?GELQF since then. In SGELSS it converted the SGEBRD, SORMBR, SORGBR and SORMLQ terms but left this one, the only workspace formula still in the file:

lapack/SRC/sgelss.f

Lines 323 to 324 in 9e518f1

MAXWRK = M + M*ILAENV( 1, 'SGELQF', ' ', M, N, -1,
$ -1 )

With the reference SGELQF the formula and the query agree, because the query returns M*NB from the same ILAENV call whenever MIN(M, N) > 0, and Path 2a is only taken then. So the LWORK that SGELSS returns does not change. Workspace queries at a few Path 2a sizes return the same LWORK before and after the change.

Replace the formula with a workspace query, as 9e96fbe did in dgelss.f, so that SGELSS no longer depends on how SGELQF sizes its workspace.

Path 2a computed the SGELQF workspace as M*ILAENV(1,'SGELQF',...),
which duplicates SGELQF's own formula. Use a workspace query instead,
as DGELSS, CGELSS and ZGELSS do. The returned LWORK is unchanged.

@langou langou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Thanks @kyungminlee

@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.33%. Comparing base (9e518f1) to head (5a956b8).
⚠️ Report is 6 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1428      +/-   ##
==========================================
- Coverage   69.36%   69.33%   -0.03%     
==========================================
  Files        6122     6122              
  Lines      486717   487029     +312     
  Branches    23268    23268              
==========================================
+ Hits       337590   337688      +98     
- Misses     148689   148903     +214     
  Partials      438      438              
Components Coverage Δ
BLAS 97.94% <ø> (ø)
CBLAS 96.98% <ø> (ø)
LAPACK 82.30% <100.00%> (-0.09%) ⬇️
LAPACKE 2.17% <ø> (ø)
TMGLIB 55.69% <ø> (ø)
BLAS testing 88.33% <ø> (ø)
CBLAS testing 89.63% <ø> (ø)
LAPACK testing 82.21% <ø> (ø)
LAPACKE testing ∅ <ø> (∅)
Files with missing lines Coverage Δ
SRC/sgelss.f 69.96% <100.00%> (+0.23%) ⬆️

... and 8 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e518f1...5a956b8. Read the comment docs.

@langou
langou merged commit b8fc4dc into Reference-LAPACK:master Sep 26, 2026
46 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants