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

Tracking issue for float_extras stabilization #27752

Closed
aturon opened this issue Aug 12, 2015 · 15 comments
Closed

Tracking issue for float_extras stabilization #27752

aturon opened this issue Aug 12, 2015 · 15 comments
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@aturon
Copy link
Member

aturon commented Aug 12, 2015

There are a number of floating point methods that are reexported from cmath, without a strong naming convention on the Rust side, or a clear rationale for which methods are included and which are not.

Ideally, someone who works with floating point, and these methods in particular, could offer advice/an RFC laying out a good vision for these methods.

@aturon aturon added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Aug 12, 2015
@pyfisch
Copy link
Contributor

pyfisch commented Sep 14, 2015

For a CBOR serializer crate I used the f32::ldexp method, I hope to push this discussion forward so my crate will eventually work on stable in the future 😁

The unstable methods of f64 are:

  • fn integer_decode(self) -> (u64, i16, i8)
  • fn ldexp(x: f64, exp: isize) -> f64
  • fn frexp(self) -> (f64, isize)
  • fn next_after(self, other: f64) -> f64

The unstable methods of f32 are:

  • fn integer_decode(self) -> (u64, i16, i8)
  • fn to_degrees(self) -> f32
  • fn to_radians(self) -> f32
  • fn ldexp(x: f32, exp: isize) -> f32
  • fn frexp(self) -> (f32, isize)
  • fn next_after(self, other: f32) -> f32

I wonder why to_degrees() and to_radians() are stable for f64 but not for f32 since both implement the same trigonometric functions so they are equally usefull on both types.

The ldexp function should use i32 instead of isize since it does not deal with memory locations. In C ldexp is defined as double ldexp (double x, int exp); so it also uses only four bytes. Since it carries the exponent the value will always be much smaller than i32::MAX. #22098 is a discussion about if ldexp should become a method or stay a function. Since all other operations for floats take self as the first argument I would change ldexp to a method.

frexp is the counterpart of ldexp and should use the same types.

I don't know what should happen with integer_decode. C does not have a corresponding function and I also do not know what the use case for this function is. Can somebody please give me a hint? It looks like a port of an integer-decode-float function from Lisp.

next_after is a rust port of the nextafter C function. I am unsure if it should be stabilized now.

@hanna-kruppe
Copy link
Contributor

ldexp and frexp are C-isms. I suspect these functions are the way they are not because it's the best design but because the C standard doesn't guarantee anything about the implementation of float and double. Therefore they did the next best thing, providing a supposedly purely arithmetic operation that matches the implementation details IEEE-like binary floating point extremely well. Take my opinion with a grain of salt, I never had to use those functions in Rust nor in C.But most people that I've heard talk about those functions, they pitch it as a way to manipulate the mantissa and the exponent directly. If that is indeed the primary use case, then I believe it is better served with something closer to integer_decode.

@pyfisch Could you elaborate what you're using ldexp for?

@pyfisch
Copy link
Contributor

pyfisch commented Oct 4, 2015

@rkruppe: I use them to decode half floats to f32: https://github.com/pyfisch/cbor/blob/master/src/de.rs#L173

@hanna-kruppe
Copy link
Contributor

Oh, so you're also using it in lieu of a proper API for disassembling and reassembling IEEE 754 floats. As I said before, I think integer_decode (after a substantial overhaul) and a hypothetical inverse integer_encode is superior for that use case. I am not wedded to the names and the tuple return type (a proper struct may be better), but in any case it should be explicit about what it does.

@haudan
Copy link

haudan commented Dec 28, 2015

Personally, I haven't written a single program, in which I wouldn't have needed the radian / degree conversion methods. Seeing the f32 versions being stabilized would be a bliss.

@aturon
Copy link
Member Author

aturon commented Dec 31, 2015

@Lisoph D'oh! That seems to just be an oversight; the ones on f64 are stable. Are you interested in writing a stabilization PR? If not, I can do it.

@haudan
Copy link

haudan commented Jan 1, 2016

@aturon I submitted an issue, was that the right thing to do? I'm new to Rust's RFC culture so I'm wondering.

@aturon
Copy link
Member Author

aturon commented Jan 1, 2016

@Lisoph That's fine -- if you'd like, you can also submit a PR to stabilize it. If not, I or someone else from the libs team will take care of it soon!

@brson
Copy link
Contributor

brson commented Jun 9, 2016

+1 to any resolution to these. Don't care much at this point about the naming question or the exact functionality. Let's just decide.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Jun 9, 2016

If the choice is between stabilizing or deprecating now, I would plead for deprecation. The current interface is sub-par, designing a good one is hard, this functionality is relatively niche and really doesn't need to be in std — it can easily be implemented with transmute and equivalent operations are already implemented in the ieee754 crate.

@alexcrichton
Copy link
Member

🔔 This issue is now entering its final comment period 🔔

The libs team was unfortunately a little late on stabilizations this cycle, but we're thinking of probably doing a backport of stabilizations partway through the beta cycle. We were somewhat ambivalent if I remember correctly on whether to stabilize or deprecate these functions. The functionality is likely needed by someone, but the names are unfortunately sub-par wrt the rest of the module.

@alexcrichton alexcrichton added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed I-nominated labels Jun 20, 2016
@pyfisch
Copy link
Contributor

pyfisch commented Jun 21, 2016

I think they should be deprecated since they are not used often enough. Maybe some will be reintroduced in the future with better names.

@alexcrichton
Copy link
Member

The libs team discussed this issue during triage yesterday and the decision was to deprecate. We realize that the FCP for this issue was pretty short, however, so please comment with any objections you might have! We're very willing to backport an un-deprecate for the few APIs we have this cycle.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 3, 2016
Although the set of APIs being stabilized this release is relatively small, the
trains keep going! Listed below are the APIs in the standard library which have
either transitioned from unstable to stable or those from unstable to
deprecated.

Stable

* `BTreeMap::{append, split_off}`
* `BTreeSet::{append, split_off}`
* `Cell::get_mut`
* `RefCell::get_mut`
* `BinaryHeap::append`
* `{f32, f64}::{to_degrees, to_radians}` - libcore stabilizations mirroring past
  libstd stabilizations
* `Iterator::sum`
* `Iterator::product`

Deprecated

* `{f32, f64}::next_after`
* `{f32, f64}::integer_decode`
* `{f32, f64}::ldexp`
* `{f32, f64}::frexp`
* `num::One`
* `num::Zero`

Added APIs (all unstable)

* `iter::Sum`
* `iter::Product`
* `iter::Step` - a few methods were added to accomodate deprecation of One/Zero

Removed APIs

* `From<Range<T>> for RangeInclusive<T>` - everything about `RangeInclusive` is
  unstable

Closes rust-lang#27739
Closes rust-lang#27752
Closes rust-lang#32526
Closes rust-lang#33444
Closes rust-lang#34152
cc rust-lang#34529 (new tracking issue)
bors added a commit that referenced this issue Jul 3, 2016
std: Stabilize APIs for the 1.11.0 release

Although the set of APIs being stabilized this release is relatively small, the
trains keep going! Listed below are the APIs in the standard library which have
either transitioned from unstable to stable or those from unstable to
deprecated.

Stable

* `BTreeMap::{append, split_off}`
* `BTreeSet::{append, split_off}`
* `Cell::get_mut`
* `RefCell::get_mut`
* `BinaryHeap::append`
* `{f32, f64}::{to_degrees, to_radians}` - libcore stabilizations mirroring past
  libstd stabilizations
* `Iterator::sum`
* `Iterator::product`

Deprecated

* `{f32, f64}::next_after`
* `{f32, f64}::integer_decode`
* `{f32, f64}::ldexp`
* `{f32, f64}::frexp`
* `num::One`
* `num::Zero`

Added APIs (all unstable)

* `iter::Sum`
* `iter::Product`
* `iter::Step` - a few methods were added to accomodate deprecation of One/Zero

Removed APIs

* `From<Range<T>> for RangeInclusive<T>` - everything about `RangeInclusive` is
  unstable

Closes #27739
Closes #27752
Closes #32526
Closes #33444
Closes #34152
cc #34529 (new tracking issue)
@jethrogb
Copy link
Contributor

jethrogb commented Jul 21, 2016

I was integer_decode to compute an integer's next-highest power of 2. I have now benchmarked it and the shifting version is significantly faster.

@lilith
Copy link

lilith commented Dec 17, 2016

next_after is extremely useful for unit testing with floats...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants