Skip to content

BLAS: Restore compact ROTMG flag output when no rescaling occurs - #1344

Merged
langou merged 1 commit into
Reference-LAPACK:masterfrom
ACSimon33:rotmg_compact_flags
Aug 1, 2026
Merged

BLAS: Restore compact ROTMG flag output when no rescaling occurs#1344
langou merged 1 commit into
Reference-LAPACK:masterfrom
ACSimon33:rotmg_compact_flags

Conversation

@ACSimon33

Copy link
Copy Markdown
Contributor

Description

Since 6dc1a78 (#1284, the fix for #244), SROTMG/DROTMG normalize
DFLAG 0/1 to -1 for every regular result, not just rescaled ones. That
contradicts the documented compact DPARAM forms (implied values "ARE NOT
STORED"), diverges from other implementations (OpenBLAS, MKL, Gonum), costs
xROTM its cheaper flag-0/1 paths, and fails xblat1's ROTMG points 1–2,
whose expected results still encode the compact forms:

SROTMG COMPUTATIONAL TESTS:       81 RUN,        6 FAILED
DROTMG COMPUTATIONAL TESTS:       81 RUN,        6 FAILED

The commit message of 6dc1a78 describes a minimal fix — change the scaling
loops' ELSE to ELSE IF (DFLAG.EQ.ONE) so the implied-element
initialization can't clobber an already-scaled DH12/DH21 — but the code
instead hoisted the normalization out of the loops and made it
unconditional. This PR implements what the message described: normalization
back inside both loops, with the guard.

6dc1a78 normalized DFLAG 0/1 to -1 for every regular result,
contradicting the documented compact DPARAM forms and failing the
xblat1 ROTMG tests at points 1-2. The ELSE IF guard alone is enough
to fix the Reference-LAPACK#244 clobber, so keep the normalization inside the loops.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 30, 2026 16:47

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@langou

langou commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Thanks.

Related, I wonder why we do not do what I suggested at: #1000 (comment). That is to replace these lines:

lapack/BLAS/SRC/drotmg.f

Lines 115 to 119 in cf47d31

* ..
* .. Data statements ..
*
DATA ZERO,ONE,TWO/0.D0,1.D0,2.D0/
DATA GAM,GAMSQ,RGAMSQ/4096.D0,16777216.D0,5.9604645D-8/

with

DOUBLE PRECISION, PARAMETER :: GAM = 2.0D0**12
DOUBLE PRECISION, PARAMETER :: GAMSQ = 2.0D0**24
DOUBLE PRECISION, PARAMETER :: RGAMSQ = 2.0D0**(-24)

@ACSimon33

Copy link
Copy Markdown
Contributor Author

No objections from my side, but should that be part of this PR? I'd create a separate PR that replaces the DATA statements with PARAMETERS.

@langou
langou merged commit 018bf9f into Reference-LAPACK:master Aug 1, 2026
16 checks passed
@langou

langou commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

@ACSimon33 wrote: "No objections from my side, but should that be part of this PR? I'd create a separate PR that replaces the DATA statements with PARAMETERS."

OKey doc. Let us do a separate PR. Good idea. I'll work on it when I can. Thanks!

Note: there are two reasons for the future PR, (1) yes, replace DATA statements with PARAMETERS, and (2) enter accurate values for the variables GAMSQ and RGAMSQ. Right now, the variables GAMSQ and RGAMSQ are approximations of their exact values. Since we can store them exactly, we might as well store them exactly as opposed to use approximations.

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.

3 participants