Skip to content

Fix documentation error and ordering bug in DLASD7 #1140

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+46 −18
Open

Fix documentation error and ordering bug in DLASD7 #1140

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d1f15f5
Fix documentation error and ordering bug in DLASD7
Jun 25, 2025
319d40e
Change new label number in DLASD7 to keep the order consistent
Jun 28, 2025
fd0ebf6
Fix documentation error and ordering bug in SLASD7
Jun 28, 2025
7c8604c
Fix documentation error and ordering bug in SLASD2 and DLASD2
Jun 28, 2025
c4ca73c
Fix documentation error in SLAED2, DLAED2, SLAED8 and DLAED8
Jun 28, 2025
fd9092e
Remove redundant/harmful ABS in SLASD7
Jun 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions SRC/dlasd7.f
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
*> On entry D contains the singular values of the two submatrices
*> to be combined. On exit D contains the trailing (N-K) updated
*> singular values (those which were deflated) sorted into
*> increasing order.
*> decreasing order.
Copy link
Contributor

@langou langou Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

(1) The change (increasing → decreasing) is great. Thanks @MaartenBaert

(2) As mentioned by @martin-frbg, it would better to report the same change to slasd7.f, slasd2.f and dlasd2.f. The same mistake is in these three files.

(3) Something to consider, I think "nonincreasing" would be better than "decreasing" in this context since we have D( I ) ≥ D(I+1), we do not necessarily have D( I ) > D(I+1). So I would suggest to change (increasing → nonincreasing).

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at [s/d]laed2.f and and [s/d]laed8.f and these four routines use "increasing". They do not use the term "nondecreasing". This is to say that the proposed change (increasing → decreasing) is more consistent with is done in [s/d]laed2.f and and [s/d]laed8.f. So please discard my comment #3 above that starts with "(3) Something to consider, I think . . ."

lapack/SRC/dlaed2.f

Line 76 in efbd2fd

*> (those which were deflated) sorted into increasing order.

lapack/SRC/dlaed8.f

Line 86 in efbd2fd

*> (those which were deflated) sorted into increasing order.

Copy link
Contributor

@langou langou Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Most drivers routines use the terminology "ascending order" for HEEV (resp. "descending order" for SVD). Most drivers routines do not use the terminology "increasing order" for HEEV (resp. "decreasing order" for SVD). (This might be more due to comment-copy-paste than intentionality comment writing.)

Some examples for the HEEV drivers:

lapack/SRC/dsyev.f

Line 86 in efbd2fd

*> If INFO = 0, the eigenvalues in ascending order.

lapack/SRC/dsyevd.f

Line 92 in efbd2fd

*> If INFO = 0, the eigenvalues in ascending order.

lapack/SRC/dsyevr.f

Line 240 in efbd2fd

*> ascending order.

lapack/SRC/dsyevx.f

Line 167 in efbd2fd

*> eigenvalues in ascending order.

Some examples for the SVD drivers:

lapack/SRC/dgesvd.f

Line 47 in efbd2fd

*> are returned in descending order. The first min(m,n) columns of

lapack/SRC/dgesvdx.f

Line 51 in efbd2fd

*> are returned in descending order. The first min(m,n) columns of

GESVDQ avoids to use any term and

lapack/SRC/dgesvdq.f

Line 156 in efbd2fd

*> The singular values of A, ordered so that S(i) >= S(i+1).

lapack/SRC/dgesvd.f

Line 121 in efbd2fd

*> The singular values of A, sorted so that S(i) >= S(i+1).

*> \endverbatim
*>
*> \param[out] Z
Expand Down Expand Up @@ -453,7 +453,7 @@ SUBROUTINE DLASD7( ICOMPQ, NL, NR, SQRE, K, D, Z, ZW, VF, VFW,
*
* Check if singular values are close enough to allow deflation.
*
IF( ABS( D( J )-D( JPREV ) ).LE.TOL ) THEN
IF( ( D( J )-D( JPREV ) ).LE.TOL ) THEN
*
* Deflation is possible.
*
Expand Down Expand Up @@ -489,7 +489,14 @@ SUBROUTINE DLASD7( ICOMPQ, NL, NR, SQRE, K, D, Z, ZW, VF, VFW,
CALL DROT( 1, VF( JPREV ), 1, VF( J ), 1, C, S )
CALL DROT( 1, VL( JPREV ), 1, VL( J ), 1, C, S )
K2 = K2 - 1
IDXP( K2 ) = JPREV
*
* Insert the deflated index in the correct position in IDXP.
* If J - JPREV is greater than 1, the indices in between
* must be shifted to preserve the correct output order.
*
DO 130 JP = JPREV, J - 1
IDXP( K2 + J - 1 - JP ) = JP
130 CONTINUE
JPREV = J
ELSE
K = K + 1
Expand Down