Skip to content
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.

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

Make lapy and nrm2 consistent #607

Merged
merged 1 commit into from
Aug 19, 2021
Merged

Conversation

angsch
Copy link
Collaborator

@angsch angsch commented Aug 8, 2021

Description

If some of the inputs are inf, {s,d}lapy{2,3} and the new {s,d}nrm2 routines return inconsistent results, NaN in contrast to inf.

  • lapy2 if both inputs are inf
  • lapy3 if at least one of the 3 inputs is inf

This commit adds a check to avoid the divisions inf / inf that introduce NaN.

Checklist

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

Copy link
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!

Copy link
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.

  1. Instead of using the FORTRAN INTRINSIC HUGE, it might be better to use DLAMCH( 'Overflow' ).
  2. Instead of calling MAXN, maybe call this variable HUGEVAL. (I agree that MAX is better to describe this variable, but HUGE is what LAPACK and FORTRAN seem to be using, so I think we want to be consistent.)
  3. Instead of using ==, it might be better to use .EQ. (for consistency throughout LAPACK)

Copy link
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.

General question. Why does LAPACK not have DISPOSINFTY ? There is DISNAN, but not DISPOSINFTY. Such that the W > MAXN of @angsch would be a DISPOSINFTY( W ). (And we can discuss on the name of this subroutine.)

Copy link
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.

Very nice PR @angsch ! "Correct" behavior of LAPACK routines under NaN and Inf is something we are working on right now, and this is a great contribution!

The first part of the current effort is to define what the "correct" behavior of LAPACK routines under NaN and Inf should be. The second part is to make the "correct" behavior happen in the routines. And finally the third and last part is to write some test that check for "correct" behavior under NaN and Inf.

Perfect timing for this contribution!

As far what the "correct" behavior should be, I certainly agree that the output of SQRT( ( +/- INF ** 2 ) + ( +/- INF ) ** 2 ) ) is better defined as an +INF than a NaN.

Avoid the introduction of NaN by divisons inf / inf
@angsch
Copy link
Collaborator Author

angsch commented Aug 8, 2021

Where should the tests go? The testing directory appears to so far not cover any auxiliary routines.

@langou
Copy link
Contributor

langou commented Aug 12, 2021

Hi @angsch, it is not clear where these tests will go. Maybe in LAPACK/TESTING proper, maybe somewhere else. I think it is great that you are identifying a problem and give a PR with a good solution. It would be good to have the related TESTING but we are not in place yet, so I think the best as far as TESTING goes is to wait. Cheers, Julien.

@langou langou merged commit 7324758 into Reference-LAPACK:master Aug 19, 2021
@angsch angsch deleted the lapy branch August 24, 2021 17:17
@julielangou julielangou added this to the LAPACK 3.10.1 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.

None yet

4 participants