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

Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion SRC/dlaed2.f
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
*> On entry, D contains the eigenvalues of the two submatrices to
*> be combined.
*> On exit, D contains the trailing (N-K) updated eigenvalues
*> (those which were deflated) sorted into increasing order.
*> (those which were deflated) sorted into decreasing order.
*> \endverbatim
*>
*> \param[in,out] Q
Expand Down
2 changes: 1 addition & 1 deletion SRC/dlaed8.f
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
*> D is DOUBLE PRECISION array, dimension (N)
*> On entry, the eigenvalues of the two submatrices to be
*> combined. On exit, the trailing (N-K) updated eigenvalues
*> (those which were deflated) sorted into increasing order.
*> (those which were deflated) sorted into decreasing order.
*> \endverbatim
*>
*> \param[in,out] Q
Expand Down
15 changes: 11 additions & 4 deletions SRC/dlasd2.f
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,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.
*> \endverbatim
*>
*> \param[out] Z
Expand Down Expand Up @@ -217,7 +217,7 @@
*> IDXQ is INTEGER array, dimension(N)
*> This contains the permutation which separately sorts the two
*> sub-problems in D into ascending order. Note that entries in
*> the first hlaf of this permutation must first be moved one
*> the first half of this permutation must first be moved one
*> position backward; and entries in the second half
*> must first have NL+1 added to their values.
*> \endverbatim
Expand Down Expand Up @@ -451,7 +451,7 @@ SUBROUTINE DLASD2( NL, NR, SQRE, K, D, Z, ALPHA, BETA, U, LDU,
*
* 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 @@ -487,7 +487,14 @@ SUBROUTINE DLASD2( NL, NR, SQRE, K, D, Z, ALPHA, BETA, U, LDU,
END IF
COLTYP( JPREV ) = 4
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 105 JP = JPREV, J - 1
IDXP( K2 + J - 1 - JP ) = JP
105 CONTINUE
JPREV = J
ELSE
K = K + 1
Expand Down
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 85 JP = JPREV, J - 1
IDXP( K2 + J - 1 - JP ) = JP
85 CONTINUE
JPREV = J
ELSE
K = K + 1
Expand Down
2 changes: 1 addition & 1 deletion SRC/slaed2.f
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
*> On entry, D contains the eigenvalues of the two submatrices to
*> be combined.
*> On exit, D contains the trailing (N-K) updated eigenvalues
*> (those which were deflated) sorted into increasing order.
*> (those which were deflated) sorted into decreasing order.
*> \endverbatim
*>
*> \param[in,out] Q
Expand Down
2 changes: 1 addition & 1 deletion SRC/slaed8.f
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
*> D is REAL array, dimension (N)
*> On entry, the eigenvalues of the two submatrices to be
*> combined. On exit, the trailing (N-K) updated eigenvalues
*> (those which were deflated) sorted into increasing order.
*> (those which were deflated) sorted into decreasing order.
*> \endverbatim
*>
*> \param[in,out] Q
Expand Down
15 changes: 11 additions & 4 deletions SRC/slasd2.f
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,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.
*> \endverbatim
*>
*> \param[out] Z
Expand Down Expand Up @@ -217,7 +217,7 @@
*> IDXQ is INTEGER array, dimension (N)
*> This contains the permutation which separately sorts the two
*> sub-problems in D into ascending order. Note that entries in
*> the first hlaf of this permutation must first be moved one
*> the first half of this permutation must first be moved one
*> position backward; and entries in the second half
*> must first have NL+1 added to their values.
*> \endverbatim
Expand Down Expand Up @@ -451,7 +451,7 @@ SUBROUTINE SLASD2( NL, NR, SQRE, K, D, Z, ALPHA, BETA, U, LDU,
*
* 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 @@ -487,7 +487,14 @@ SUBROUTINE SLASD2( NL, NR, SQRE, K, D, Z, ALPHA, BETA, U, LDU,
END IF
COLTYP( JPREV ) = 4
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 105 JP = JPREV, J - 1
IDXP( K2 + J - 1 - JP ) = JP
105 CONTINUE
JPREV = J
ELSE
K = K + 1
Expand Down
13 changes: 10 additions & 3 deletions SRC/slasd7.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.
*> \endverbatim
*>
*> \param[out] Z
Expand Down Expand Up @@ -453,7 +453,7 @@ SUBROUTINE SLASD7( 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 SLASD7( ICOMPQ, NL, NR, SQRE, K, D, Z, ZW, VF, VFW,
CALL SROT( 1, VF( JPREV ), 1, VF( J ), 1, C, S )
CALL SROT( 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 85 JP = JPREV, J - 1
IDXP( K2 + J - 1 - JP ) = JP
85 CONTINUE
JPREV = J
ELSE
K = K + 1
Expand Down