comments fixed: op missed

[jip]

Error in xQRT17 comments fixed.

[codecov]

Codecov Report

Merging #562 (8531d65) into master (e7bb5f4) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #562   +/-   ##
=======================================
  Coverage   82.37%   82.37%           
=======================================
  Files        1894     1894           
  Lines      190677   190677           
=======================================
  Hits       157063   157063           
  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 e7bb5f4...8531d65. Read the comment docs.

[langou]

Good catch. Thanks @jip. I agree. op(A), not A. You are correct.

Well, then I ended up reading this routine. LOL.

One comments then.
I thought it would be good to state the norm used by the routine in the comments. I read:

CQRT17 computes the ratio

    || R^H * op(A) || / ( ||A|| * alpha * max(M,N,NRHS) * eps )

where R = op(A) * X - B, op(A) is A or A^H, and

    alpha = || B || if IRESID = 1 (zero-residual problem)
    alpha = || R || if IRESID = 2 (otherwise).

Great. Which norm?

Then I see that the code uses the 1-norm.

Then this line caught my eyes:

      NORMA = CLANGE( 'One-norm', M, N, A, LDA, RWORK )

And then I was not sure whether we should take the 1-norm of A or the 1-norm of op(A).

If we add "all norms are 1-norm" then the comments and code would be correct. (They already were, but that's just to clarify.) And that would be that.

Although I am not sure whether we should take the 1-norm of A or the 1-norm of op(A) in this context.

Oh well. The code takes the 1-norm of A, that’s fine with me.

( If we want the 1-norm of op(A), we can simply take the Inf-norm of A and that’ll do the trick. )

[jip]

Routines xGTT02, xTBT02, xTPT02, xTRT02 use 1-norm of op(A). Since norm(A) == norm(op(A)) for PB, PO, PP, PT, SP and SY cases, the same is true for xPBT02, xPOT02, xPPT02, xPTT02, xSPT02 and xSYT02 routines, too.

So it may be seen that xxxT02 testing routines excepting xGBT02 and xGET02 use 1-norm of op(A) not just A. This contradicts with approach in xQRT17 which uses 1-norm of A not op(A). xGBT02 and xGET02 routines use 1-norm of A.

I could make a PR to arrange this but not sure it's an error.

By the way, comments in xGTT02 declare that it uses norm(A) but its code implements 1-norm of op(A).

[weslleyspereira]

Hi @jip. Thanks for clarifying.

"Error" is a big word. Maybe it’s a "feature"! :).

In any case, I think we want (a) to have correct comments (so the comments should say norm(op(A)) when we use norm(op(A))), (b) to have more comments (specify the norm used), (c) be consistent (so compute op(A) for all these testings subroutines.)

I could make a PR to arrange this

1. Would you like to propose a PR with additional comments on those routines? Indeed adding something like "The norm used is the 1-norm." at the end of the "Purpose" section would be great. And that should be enough.

2. "xGBT02 and xGET02 routines use 1-norm of A." It would be more consistent to use 1-norm of op(A). Do you want to propose a PR with these changes? (Comments and code.) I think it is more consistent, and mathematically, I do not think it matters much, but I think op(A) is better.

3. "By the way, comments in xGTT02 declare that it uses norm(A) but its code implements 1-norm of op(A)." OK. So this needs to be changed too then. Do you want to propose a PR with this change?

[jip]

Yes, I do. I'll prepare 3 separate PRs.