Skip to content

Conversation

@MattStephanson
Copy link
Contributor

Fixes #1059.

  • Need to introduce a floating point to avoid integer truncation.
  • The exp(-x) term needs to be smaller than ULP/2, which requires x > (p+1)*ln(2)/2. But NBITS is the number of physical, not effective, bits, so (NBITS+2) is needed. Could also use numeric_limits<T>::digits+1.

@MattStephanson MattStephanson requested a review from a team as a code owner August 6, 2020 19:40
@ghost
Copy link

ghost commented Aug 6, 2020

CLA assistant check
All CLA requirements met.

@StephanTLavavej StephanTLavavej added the bug Something isn't working label Aug 8, 2020
@StephanTLavavej
Copy link
Member

Thanks! We're a bit busy at the moment with VS 2019 16.8 Preview 3 but we'll try to review your PRs soon. 😸

Copy link
Contributor

@cbezault cbezault left a comment

Choose a reason for hiding this comment

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

I'm convinced that this is correct but could you add a (very) small test for this change? It could be as simple as using the examples that @statementreply put in #1059 as a sanity check.

 - Fix integer truncation and off-by-one errors in calculating limit where
   exp(-x) can be ignored in hyperbolic functions.
@MattStephanson
Copy link
Contributor Author

I'm convinced that this is correct but could you add a (very) small test for this change? It could be as simple as using the examples that @statementreply put in #1059 as a sanity check.

Added test case. Sorry if my rebase messed things up, I can reset to fc95b36 if need be.

Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good to me! I'll push a couple of minor test changes.

@StephanTLavavej StephanTLavavej merged commit 465d0a5 into microsoft:master Aug 22, 2020
@StephanTLavavej
Copy link
Member

Thanks for fixing these long-standing correctness bugs, and congratulations on your first microsoft/STL commit! 😺 🚀

@MattStephanson MattStephanson deleted the hyperbolic_limit branch September 29, 2020 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect const float values in xvalues.cpp files

4 participants