-
Notifications
You must be signed in to change notification settings - Fork 128
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
Use constant for 180/π in f32::to_degrees #61
Conversation
The current `f32::to_degrees` implementation uses a division to calculate 180/π, which causes a loss of precision. Using a constant is still not perfect (implementing a maximally-precise algorithm would come with a high performance cost), but improves precision with a minimal change. This is a backport from [`std`]. [`std`]: rust-lang/rust@e34c31b
Just for reference, we also discussed the implication that some inputs are now less accurate, in rust-lang/rust#48617. But the rounding is more correct in general, and here we forward to |
src/float.rs
Outdated
|
||
#[test] | ||
fn to_degrees_rounding() { | ||
assert_eq!(1_f32.to_degrees(), 57.2957795130823208767981548141051703); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we forward to std
, this fails on Rust 1.8 since it lacks rust-lang/rust#47919. Maybe just limit this test to no-std builds? (with a comment explaining this reason)
Strange, 1.8 still failed!
|
Solved by explicitly calling bors r+ |
61: Use constant for 180/π in f32::to_degrees r=cuviper a=vks The current `f32::to_degrees` implementation uses a division to calculate 180/π, which causes a loss of precision. Using a constant is still not perfect (implementing a maximally-precise algorithm would come with a high performance cost), but improves precision with a minimal change. This is a backport from [`std`]. [`std`]: rust-lang/rust@e34c31b Co-authored-by: Vinzent Steinberg <vinzent.steinberg@gmail.com> Co-authored-by: Josh Stone <cuviper@gmail.com>
Thanks for the fix! I forgot about this PR... |
Build succeeded |
The current
f32::to_degrees
implementation uses a division tocalculate 180/π, which causes a loss of precision. Using a constant is
still not perfect (implementing a maximally-precise algorithm would come
with a high performance cost), but improves precision with a minimal
change.
This is a backport from
std
.