Skip to content

clargv,zlargv: rename local ABS1 helper to CABSMAX#1220

Merged
langou merged 1 commit intoReference-LAPACK:masterfrom
nakatamaho:topic/xLARGV-fix-helper-naming
Mar 28, 2026
Merged

clargv,zlargv: rename local ABS1 helper to CABSMAX#1220
langou merged 1 commit intoReference-LAPACK:masterfrom
nakatamaho:topic/xLARGV-fix-helper-naming

Conversation

@nakatamaho
Copy link
Copy Markdown
Contributor

The local ABS1 statement function in CLARGV and ZLARGV does not compute the usual complex ABS1/CABS1 quantity. It returns

max(abs(real(z)), abs(aimag(z)))

and is used only as a scaling helper in the same algorithmic path as CLARTG/ZLARTG.

Rename the helper to CABSMAX so that the name matches its actual behavior and does not suggest CABS1 semantics.

No numerical behavior is changed.

Description

Checklist

  • The documentation has been updated.
  • If the PR solves a specific issue, it is set to be closed on merge.

The local ABS1 statement function in CLARGV and ZLARGV does not compute
the usual complex ABS1/CABS1 quantity. It returns

    max(abs(real(z)), abs(aimag(z)))

and is used only as a scaling helper in the same algorithmic path as
CLARTG/ZLARTG.

Rename the helper to CABSMAX so that the name matches its actual
behavior and does not suggest CABS1 semantics.

No numerical behavior is changed.
nakatamaho added a commit to nakatamaho/mplapack that referenced this pull request Mar 28, 2026
…k#1219, #1220)

Upstream lapack#1219 renames the local ABS1 statement function to CABS1
in BLAS/LAPACK/CBLAS sources where it computes |Re(z)| + |Im(z)|.
Upstream lapack#1220 further handles Clargv/Zlargv, whose local ABS1
helper actually computes max(|Re(z)|, |Im(z)|) and must not be renamed
to CABS1; rename it to CABSMAX instead.

Apply both to the MPLAPACK C++ codebase:

- Add patch-cabs1_unification and patch-cabsmax to
  external/lapack/Makefile.am.
- Update mplapack/reference/Clargv.cpp: add inline cabsmax(), replace
  the two cabs1() call sites with cabsmax().
- Regenerate fable/3.12.1/lapack/patch-Clargv.cpp: introduce cabsmax
  inline helper and drop the erroneous lvalue ABS1 statement removal.
- Remove now-superfluous patches from fable/patch_lapack_3.12.1.sh:
  Cggev, Cggev3, Cggevx, Ctgevc, Clags2, Chgeqz were previously
  patched for ABS1->CABS1 and are subsumed by lapack#1219.
- Remove from fable/patch_lapack_test_3.12.1.sh: Cdrges, Cdrges3,
  Cget52 for the same reason.
- Simplify patch-Cdrgsx.cpp: the abs1->cabs1 hunks are no longer
  needed after lapack#1219.

No numerical change.

cf. Reference-LAPACK/lapack#1220
Reference-LAPACK/lapack#1219
Copy link
Copy Markdown
Contributor

@langou langou left a comment

Choose a reason for hiding this comment

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

Thanks! Yes, that'll avoid confusion for whoever thinks they know what CABS1 is. CABSMAX is a good choice of name WRT CABS1. Thanks!

@langou langou merged commit d3388ad into Reference-LAPACK:master Mar 28, 2026
12 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