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

RFC: numerics reform #369

Merged
merged 4 commits into from
Nov 4, 2014
Merged

RFC: numerics reform #369

merged 4 commits into from
Nov 4, 2014

Conversation

aturon
Copy link
Member

@aturon aturon commented Oct 8, 2014

This RFC is preparation for API stabilization for the std::num module. The goal is to finish the simplification efforts started in @bjz's reversal of the numerics hierarcy.

Broadly, the proposal is to collapse the remaining numeric hierarchy in std::num, and to provide only limited support for generic programming (roughly, only over primitive numeric types that vary based on size). Traits giving detailed numeric hierarchy can and should be provided separately through the Cargo ecosystem.

Thus, this RFC proposes to flatten or remove most of the traits currently provided by std::num, and generally to simplify the module as much as possible in preparation for API stabilization.

Rendered

@Gankra
Copy link
Contributor

Gankra commented Oct 8, 2014

Earlier drafts of this work were based on the listed alternative: more aggressive inlining of traits. iirc it was down to Float and Int, with SignedInt and UnsignedInt as potentials. FloatMath surviving is clearly motivated by the RFC, but what about the rest? Was it about deduplication and avoiding the more dubious methods like "abs on uints", or is this just a path of least resistance solution?

I wasn't under the impression that we were particularly interested in supporting using a generic "Num".

@aturon
Copy link
Member Author

aturon commented Oct 8, 2014

@gankro Thanks for bringing that up. I've revised the RFC with a new paragraph (see "Overview: the new hierarchy") to explain the motivation.

I think flattening Num away would be fine, and maybe the best move to make in terms of hitting the desired perception. Getting rid of Signed and UnsignedInt would be a deeper change.

@aturon
Copy link
Member Author

aturon commented Oct 8, 2014

@gankro on further reflection, I've decided that the original plan (without Num) is likely the better one. I've revised the RFC accordingly, but left the definition of Num in Alternatives for comparison.

@Gankra
Copy link
Contributor

Gankra commented Oct 8, 2014

👍 this strikes me as a moderate compromise between the two extremes.

(I still think the signed/unsigned separation is a bit silly, but the majority seems to disagree)

@jsanders
Copy link

jsanders commented Oct 8, 2014

@aturon: Do you have any links where I can read about the ideas for replacing FromPrimitive and friends?

@aturon
Copy link
Member Author

aturon commented Oct 8, 2014

@jsanders See this proposal and the discussion on this issue about Ordinal.

@aturon
Copy link
Member Author

aturon commented Oct 8, 2014

Added these links to the RFC.

@Gankra
Copy link
Contributor

Gankra commented Oct 8, 2014

Was having just Float, SignedInt, and UnsignedInt (and FloatMath as per discussion) ever evaluated? This is a denormalization that has more duplication, but avoids all the "this operation makes no sense" trouble. It also neatly matches what the primitive families actually are.

@aturon
Copy link
Member Author

aturon commented Oct 8, 2014

@gankro Yes, I thought about that possibility. The updated RFC gives a (somewhat weak) justification for Signed: we want those methods in the prelude, because unlike most of the other methods, they are truly basic operations that one expects to be present whenever working with a signed numeric type.

Also, while it's a bit far-fetched to imagine useful code that's generic over both ints and floats, being generic over signedness of ints is not so hard to imagine. (Note that the RFC does make some amount of generic programming over natural groupings of the primitive types an explicit goal.)

@Gankra
Copy link
Contributor

Gankra commented Oct 8, 2014

Ah, I didn't process that only some of these would be part of the prelude. Okay, that makes sense.

@klutzy
Copy link

klutzy commented Oct 9, 2014

Are the numeric traits expected to be implemented outside of std or not? For example, is it reasonable if I implement custom i31 or f80 type?
In any case, it would be good to specify actual semantic of traits, e.g. UnsignedInt is only for mod 2^N arithmetic, to reduce confusion.

Currently Signed have some methods which have different edge cases for integers and floats. For example, Signed::abs() specifies both overflow behavior of primitive integers (int::MIN.abs()) and NaN behavior of floating numbers. Is it ok to preserve the trait with the specification?

My main concern is that signed integers have asymmetric range so generic functions over Signed may introduce overflow bugs. For example, we have overflow error for num::rational.

@aturon
Copy link
Member Author

aturon commented Oct 9, 2014

@klutzy

Are the numeric traits expected to be implemented outside of std or not? For example, is it reasonable if I implement custom i31 or f80 type?

It's not an explicit design goal, but for custom-sized "primitive" types it would probably make sense.

Currently Signed have some methods which have different edge cases for integers and floats. For example, Signed::abs() specifies both overflow behavior of primitive integers (int::MIN.abs()) and NaN behavior of floating numbers. Is it ok to preserve the trait with the specification?

The goal with Signed was not to support generic int/float programming, but rather to pull those methods out into something we could have in the prelude (so that they are always available). But we could instead merge the trait into Float and a SignedInt trait, and come up with a different solution for the prelude (or simply not include this functionality there).

I would welcome further input on the topic of what needs to go into the prelude. I worry that needing to import SignedInt in order to get the abs method will be very surprising. And on the other hand we don't want to include the full traits in the prelude, because we want to leave these names open for external numerics libraries.

@Gankra
Copy link
Contributor

Gankra commented Oct 9, 2014

Maybe we should be evaluating a way to mark a trait as #[not-generic]? As in, "this is just an extension trait, being generic over it is not what you should be doing" and "this isn't intended to be implemented by custom types". Maybe a warn with an #[allow]?

@aturon
Copy link
Member Author

aturon commented Oct 9, 2014

@gankro It's an interesting thought. It could also be done via naming conventions. (Cf the -Prelude suffix proposed here)

fn is_infinite(self) -> bool;
fn is_finite(self) -> bool;
fn is_normal(self) -> bool;
fn classify(self) -> FPCategory;
Copy link
Member

Choose a reason for hiding this comment

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

Could we rename FPCategory to FloatCategory?

@brendanzab
Copy link
Member

Thanks a bunch for drawing up best proposal for Rust numerics yet!

@brendanzab
Copy link
Member

cc. @sebcrozet @SiegeLord

@brendanzab
Copy link
Member

So in the docs should we be specific about what types should implement these traits? For example, I am guessing they would not be implemented by big integers and floats...?

@kennytm
Copy link
Member

kennytm commented Oct 12, 2014

I have some code that relies only on field operations (Add + Sub + Mul + Div + Zero + One + Clone), and also a type to represent finite-field arithmetic (GF(2^64)) that Rem and the bitwise operations do not make sense.

What should be done if this RFC is to be merged? Since it seems Zero and One are going away?

@Gankra
Copy link
Contributor

Gankra commented Oct 12, 2014

@kennytm Nothing prevents you from writing these traits in your own library, which I believe is the overall intent of this RFC: Make algebraic hierarchies a problem for the community to work out in the cargo ecosystem.

@brendanzab
Copy link
Member

I am thinking it might be wise to start a numeric lib with things like the additive and multiplicative identities and such. Then folks have a place to turn once we kill some of these numeric traits. I began num-rs quite some time ago... maybe there is something we can salvage from there?

fn exp2(self) -> Self;
fn ln(self) -> Self;
fn log2(self) -> Self;
fn log10(self) -> Self;
Copy link
Member

Choose a reason for hiding this comment

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

Some of these have worried me in the past in the sense that although they are LLVM intrinsics, they're often lowered to function calls at compile-time into libraries like libm. By providing these functions in libcore we're semi-stating that they're available for everyone forever (some of them do have x86 instructions, for example), but we're not necessarily delivering on that statement.

A small wory, just wanted to point out why these are singled out for being in libcore rather than libstd. Anyone using these functions and libcore directly will likely already know how to get them anyway

@ruuda
Copy link

ruuda commented Nov 16, 2014

I am sorry for the late response. I’m only dealing with this now because the changes broke a project of mine. I understand the rationale behind the separation of Float and FloatMath, but from a usability point of view, the distinction is quite arbitrary. I have to use std::num::Float to be able to use sqrt(), but use std::num::FloatMath for cos()? Right now, I see no way of knowing whether a function is in Float or FloatMath except looking it up in the documentation. Could this be made more intuitive without changing the dependencies?

@aturon
Copy link
Member Author

aturon commented Nov 16, 2014

@ruud-v-a Thanks for the feedback! We're actually considering making std export a single Float trait that includes all of the methods. Stay tuned for additional changes in this space soon.

aturon added a commit to aturon/rust that referenced this pull request Nov 19, 2014
This commit adds stability markers for the APIs that have recently been
aligned with [numerics
reform](rust-lang/rfcs#369). For APIs that were
changed as part of that reform, `#[unstable]` is used to reflect the
recency, but the APIs will become `#[stable]` in a follow-up pass.

In addition, a few aspects of the APIs not explicitly covered by the RFC
are marked here -- in particular, constants for floats.

This commit does not mark the `uint` or `int` modules as `#[stable]`,
given the ongoing debate out the names and roles of these types.

Due to some deprecation (see the RFC for details), this is a:

[breaking-change]
aturon referenced this pull request in aturon/rust Nov 19, 2014
bors added a commit to rust-lang/rust that referenced this pull request Nov 20, 2014
…ichton

This commit adds stability markers for the APIs that have recently been aligned with [numerics reform](rust-lang/rfcs#369). For APIs that were changed as part of that reform, `#[unstable]` is used to reflect the recency, but the APIs will become `#[stable]` in a follow-up pass.

In addition, a few aspects of the APIs not explicitly covered by the RFC are marked here -- in particular, constants for floats.

This commit does not mark the `uint` or `int` modules as `#[stable]`, given the ongoing debate out the names and roles of these types.

Due to some deprecation (see the RFC for details), this is a:

[breaking-change]

r? @alexcrichton 
cc @bjz
@andydude
Copy link

The only code associated with this comment are the traits below:

pub trait RotateLeft<RHS> {
    type Output;

    fn rotate_left(self, rhs: RHS) -> Self::Output;
}

pub trait RotateRight<RHS> {
    type Output;

    fn rotate_right(self, rhs: RHS) -> Self::Output;
}

pub trait SwapBytes<RHS> {
    fn swap_bytes(self) -> Self;

    fn from_be(x: Self) -> Self;
    fn from_le(x: Self) -> Self;
    fn to_be(self) -> Self;
    fn to_le(self) -> Self;
}

I would like to bring attention to the particular use case of using these subsets of the current Int trait implemented for std::simd types (like i16x8, i32x4, i64x2, u16x8, u32x4, u64x2). The reason why you can't implement Int for SIMD types is that even though NumCast is easy to implement as the broadcast operation (a SIMD type composed of 8 copies of the same value), the reverse operations in ToPrimitive cannot be implemented in a satisfactory way (You could take the first element, or the last element, but neither of these are attractive options). So there is no natural implementation of ToPrimitive for SIMD types.

A second reason why these method subsets are useful to separate from the Int trait is that the current rotate_left and rotate_right methods strictly enforce RHS=usize, which is typically how they are used, but there are algorithms that require these methods where RHS=Self (meaning each SIMD component is rotated a different amount). This is already expressible in the current Shl and Shr traits, which are intimately connected to the rotation methods. Imaging Self below replaced with u32x4 in the following example specializations:

impl Shl<usize, Output=Self> for ... { ... }
impl Shl<Self, Output=Self> for ... { ... }
impl Shr<usize, Output=Self> for ... { ... }
impl Shr<Self, Output=Self> for ... { ... }
impl RotateLeft<usize, Output=Self> for ... { ... }
impl RotateLeft<Self, Output=Self> for ... { ... }
impl RotateRight<usize, Output=Self> for ... { ... }
impl RotateRight<Self, Output=Self> for ... { ... }

After some experimentation, it seems that Shl<Self, Self> and Shr<Self, Self> are already implemented for all SIMD types. It would be a great benefit for SIMD users if the other, more common shift and rotation methods were also built-in.

A side note (and perhaps outside the scope of this comment) is that RotateLeft and RotateRight could be renamed Rol and Ror as they can be implemented in single instructions on some architectures, even though they're not traditional C operations. Evidence can be found on the LLVM mailing list that others who have also considered them core operations.

In conclusion, there are at least 2 compelling reasons (the ToPrimitive reason and the RotateLeft reason) to separate out rotation operations from the Int trait so that SIMD types may benefit from sharing the same methods as primitive types.

cuviper pushed a commit to cuviper/rayon that referenced this pull request Mar 28, 2017
This implements a considerable portion of rust-lang/rfcs#369 (tracked in #18640). Some interpretations had to be made in order to get this to work. The breaking changes are listed below:

[breaking-change]

- `core::num::{Num, Unsigned, Primitive}` have been deprecated and their re-exports removed from the `{std, core}::prelude`.
- `core::num::{Zero, One, Bounded}` have been deprecated. Use the static methods on `core::num::{Float, Int}` instead. There is no equivalent to `Zero::is_zero`. Use `(==)` with `{Float, Int}::zero` instead.
- `Signed::abs_sub` has been moved to `std::num::FloatMath`, and is no longer implemented for signed integers.
- `core::num::Signed` has been removed, and its methods have been moved to `core::num::Float` and a new trait, `core::num::SignedInt`. The methods now take the `self` parameter by value.
- `core::num::{Saturating, CheckedAdd, CheckedSub, CheckedMul, CheckedDiv}` have been removed, and their methods moved to `core::num::Int`. Their parameters are now taken by value. This means that
- `std::time::Duration` no longer implements `core::num::{Zero, CheckedAdd, CheckedSub}` instead defining the required methods non-polymorphically.
- `core::num::{zero, one, abs, signum}` have been deprecated. Use their respective methods instead.
- The `core::num::{next_power_of_two, is_power_of_two, checked_next_power_of_two}` functions have been deprecated in favor of methods defined a new trait, `core::num::UnsignedInt`
- `core::iter::{AdditiveIterator, MultiplicativeIterator}` are now only implemented for the built-in numeric types.
- `core::iter::{range, range_inclusive, range_step, range_step_inclusive}` now require `core::num::Int` to be implemented for the type they a re parametrized over.
oceanlewis added a commit to oceanlewis/deeplearn-rs that referenced this pull request Aug 18, 2017
@Centril Centril added the A-arithmetic Arithmetic related proposals & ideas label Nov 23, 2018
djrenren pushed a commit to djrenren/libtest that referenced this pull request Jan 22, 2019
This implements a considerable portion of rust-lang/rfcs#369 (tracked in #18640). Some interpretations had to be made in order to get this to work. The breaking changes are listed below:

[breaking-change]

- `core::num::{Num, Unsigned, Primitive}` have been deprecated and their re-exports removed from the `{std, core}::prelude`.
- `core::num::{Zero, One, Bounded}` have been deprecated. Use the static methods on `core::num::{Float, Int}` instead. There is no equivalent to `Zero::is_zero`. Use `(==)` with `{Float, Int}::zero` instead.
- `Signed::abs_sub` has been moved to `std::num::FloatMath`, and is no longer implemented for signed integers.
- `core::num::Signed` has been removed, and its methods have been moved to `core::num::Float` and a new trait, `core::num::SignedInt`. The methods now take the `self` parameter by value.
- `core::num::{Saturating, CheckedAdd, CheckedSub, CheckedMul, CheckedDiv}` have been removed, and their methods moved to `core::num::Int`. Their parameters are now taken by value. This means that
- `std::time::Duration` no longer implements `core::num::{Zero, CheckedAdd, CheckedSub}` instead defining the required methods non-polymorphically.
- `core::num::{zero, one, abs, signum}` have been deprecated. Use their respective methods instead.
- The `core::num::{next_power_of_two, is_power_of_two, checked_next_power_of_two}` functions have been deprecated in favor of methods defined a new trait, `core::num::UnsignedInt`
- `core::iter::{AdditiveIterator, MultiplicativeIterator}` are now only implemented for the built-in numeric types.
- `core::iter::{range, range_inclusive, range_step, range_step_inclusive}` now require `core::num::Int` to be implemented for the type they a re parametrized over.
wycats pushed a commit to wycats/rust-rfcs that referenced this pull request Mar 5, 2019
…erability

Deprecate Computed Overridability and `.readOnly()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-arithmetic Arithmetic related proposals & ideas
Projects
None yet
Development

Successfully merging this pull request may close these issues.