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

Issue in sin/cos_pi: #737

Closed
jzmaddock opened this issue Jan 8, 2022 · 1 comment
Closed

Issue in sin/cos_pi: #737

jzmaddock opened this issue Jan 8, 2022 · 1 comment

Comments

@jzmaddock
Copy link
Collaborator

Lurking in my inbox:

I sent a long long time ago (for boost 1.72) a proposal of correction of a very small bug of the cos_pi and sin_pi implementations

and I have just see that in the last 1.78 beta 1 nothing has been corrected yet.

The bug is from a conversion, line 36 for cos_pi.hpp and line 41 for sin_pi.hpp:

if(iconvert(rem, pol) & 1)
invert = !invert;

if rem is not representable in the integral type issued by iconvert, the result can be incorrect.

I proposed a a one liner correction, replacing the wrong test by

if(floor(rem/2)*2 != rem)
invert = !invert;

which does not make use of integer conversion.

Of course any other way to correctly compute the parity of rem will do the job.

Looking forward for your answer and correction perhaps for boost 1.79 ?

Thanks for the great boost::math, and apologies for this direct mailing.

Jean-Thierry Lapresté 
mborland added a commit to mborland/math that referenced this issue Jan 9, 2022
jzmaddock added a commit that referenced this issue Jan 11, 2022
@jzmaddock
Copy link
Collaborator Author

Fixed in referenced PR

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

No branches or pull requests

1 participant