Skip to content

Conversation

@weslleyspereira
Copy link
Collaborator

@weslleyspereira weslleyspereira commented May 18, 2021

Description

Fixes #554. This PR:

  1. Correct some typos in the comments of ssml and sbig in la_constants.f90.
  2. Correct the formulas for ssml, using the ones from https://doi.org/10.1145/3061665
  3. Correct the formulas for sbig, using the ones from James Blue https://doi.org/10.1145/355769.355771 [edited]

* The ssml from https://doi.org/10.1145/3061665 is sufficient to scale both normalized and denormalized floating-point numbers.

@weslleyspereira weslleyspereira marked this pull request as ready for review May 18, 2021 23:03
@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #559 (4b510ef) into master (5f9e442) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #559   +/-   ##
=======================================
  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 5f9e442...4b510ef. Read the comment docs.

langou
langou previously approved these changes May 18, 2021
! ssml = 1/S, where S was defined in https://doi.org/10.1145/355769.355771
(minexponent(0._dp) - digits(0._dp)) * 0.5_dp))
! sbig = 1/S, where S was defined in https://doi.org/10.1145/355769.355771
real(dp), parameter :: dsbig = real(radix(0._dp), dp)**( - ceiling( &
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still unsure that this formula is correct. It gives dsbig=5.0052077379577523E-147 while the Anderson's code uses the Blue's formula which gives 1.1113793747425387E-162. Further, some test cases in my Go port overflow with this dsbig value but do not with Anderson's value. It seems that the part - digits(0._dp) + 1 should be + digits(0._dp) - 1 to reproduce the value from Anderson's code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right! I will fix it. Thanks again!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @vladimir-ch. Could you please review my changes once more. I think now everything is fine. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, LGTM. Thanks!

@weslleyspereira weslleyspereira merged commit e7bb5f4 into Reference-LAPACK:master May 21, 2021
christoph-conrads pushed a commit to christoph-conrads/lapack that referenced this pull request May 23, 2021
weslleyspereira added a commit to weslleyspereira/lapack that referenced this pull request Feb 15, 2022
…formance Libraries!

This is the same bug fixed on Reference-LAPACK#559 for la_constants.f90.
@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.

Blue's scaling parameters ssml and sbig implemented incorrectly?

4 participants