Skip to content

Conversation

@jip
Copy link
Contributor

@jip jip commented May 30, 2021

  1. Comments improved, unified and fixed.
  2. Code improved and fixed.

To unify with routines xGBT01, xGTT02, xLANGB, xTBT02, xTPT02, and xTRT02.

Some remarks:

  1. It turned out that xLANGB, xLANGT, xLANTB, and xLANTP subroutines accept only a square matrix A, unlike xLANGE and xLANTR which allow A to be non-square.
  2. xLANGB uses inline loop not SCSUM1/DZSUM1 for 1-norm and inf-norm computing.
  3. xGET02 uses magnitude-based norms |y| unlike xGBT02 which uses taxicab-based norms |Re(y)| + |Im(y)|.

jip added 4 commits May 30, 2021 17:52
The RWORK dimension was understated.
If M=1 or M<N then an index expression I2-I1+1 may take a non-positive value.
This will raise an out of bound error.

A check was borrowed from xGBT01.
…eference-LAPACK#562)

1. Comments improved, unified and fixed.
2. Code changed.

To unify with routines xGBT01, xGTT02, xTBT02, xTPT02, and xTRT02.
@jip jip changed the title modifies xGBT02 and xGET02 to use 1-norm of op(A) not A (no.2 from PR #562) modify xGBT02 and xGET02 to use 1-norm of op(A) not A (no.2 from PR #562) May 30, 2021
@codecov
Copy link

codecov bot commented May 30, 2021

Codecov Report

Merging #571 (036ec87) into master (bd6add2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #571   +/-   ##
=======================================
  Coverage   82.37%   82.37%           
=======================================
  Files        1894     1894           
  Lines      190679   190679           
=======================================
  Hits       157065   157065           
  Misses      33614    33614           

Continue to review full report at Codecov.

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

@langou
Copy link
Contributor

langou commented May 30, 2021

xGET02 uses magnitude-based norms |y| unlike xGBT02 which uses taxicab-based norms |Re(y)| + |Im(y)|.

Good point. You are correct.

The fact that we use two different measures is awkward and I do not know why this has been like that.

I wonder whether using xLANGB in xGBT02 would not (1) simplify the code of xGBT02, and (2) use magnitude-based norms |y|.

@jip
Copy link
Contributor Author

jip commented May 30, 2021

I wonder whether using xLANGB in xGBT02 would not (1) simplify the code of xGBT02, and (2) use magnitude-based norms |y|.

It would, but this change will require to extend xLANGB interface to accept non-square matrices. And also to extend xLANGT, xLANTB, and xLANTP in the same way, to maintain consistency.

@langou
Copy link
Contributor

langou commented May 31, 2021

I see. Thanks @jip for explaining.

Another idea is to have SCASUM (and DZASUM) that works with complex ABS( z ) instead of ABS(REAL( z )) + ABS(IMAG( z )). Having such SCASUM and DZASUM could actually be useful for a variety of reason. Then we would simply replace the calls to SCASUM (and DZASUM) in CGBT02 (and ZGBT02) by these new routines.

So, well, I think that's great work @jip and we can leave it at that for now. And revisit as needed in the future.

@langou langou merged commit effe175 into Reference-LAPACK:master May 31, 2021
jip added a commit to jip/lapack that referenced this pull request Jun 9, 2021
…PACK#562)

Despite the recommendation, no change has been made in xGBT02, xGET02, xGTT02, xTBT02, xTPT02,
and xTRT02 for C,Z datatypes yet: as it was noted afterwards in PR Reference-LAPACK#571, those routines are
using mixed types of norm: 1-norm for op(A) and taxicab-based norm |Re(z)|+|Im(z)| for residual.
@julielangou julielangou added this to the LAPACK 3.10.0 milestone Nov 12, 2022
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