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

Flatten numeric trait hierarchy and allow for user extensibility. #10387

Closed
brendanzab opened this issue Nov 9, 2013 · 34 comments
Closed

Flatten numeric trait hierarchy and allow for user extensibility. #10387

brendanzab opened this issue Nov 9, 2013 · 34 comments
Assignees
Labels
A-trait-system Area: Trait system
Milestone

Comments

@brendanzab
Copy link
Member

I am growing increasingly concerned that we are locking ourselves into a substandard numeric API heading toward 1.0 that will be hard to alter in the future without breaking a significant amount of client code.

Perhaps we could investigate simplifying the numeric trait hierarchy to be centered around built-in types. Perhaps the traits could be removed from the prelude as default instead using free, method wrapper functions as the primary form of using the numeric functions. This would make the API more amenable to being extended by users in the future. Perhaps an algebraic hierarchy could be included in extra.

For example the Round, Fractional, Algebraic, Trigonometric, Exponential, Hyperbolic, Real and Float could be consolidated into a single Float trait. Then the primary way of accessing the functions would be via the wrapper functions, eg. num::atan2(x, y).

pub trait Float {
    fn floor(x: Self) -> Self;
    fn ceil(x: Self) -> Self;
    fn round(x: Self) -> Self;
    fn trunc(x: Self) -> Self;
    fn fract(x: Self) -> Self;

    fn recip(x: Self) -> Self;

    fn pow(x: Self, n: Self) -> Self;
    fn sqrt(x: Self) -> Self;
    fn rsqrt(x: Self) -> Self;
    fn cbrt(x: Self) -> Self;
    fn hypot(x: Self, other: Self) -> Self;

    fn sin(x: Self) -> Self;
    fn cos(x: Self) -> Self;
    fn tan(x: Self) -> Self;

    fn asin(x: Self) -> Self;
    fn acos(x: Self) -> Self;
    fn atan(x: Self) -> Self;

    fn atan2(x: Self, y: Self) -> Self;
    fn sin_cos(x: Self) -> (Self, Self);

    fn sinh(x: Self) -> Self;
    fn cosh(x: Self) -> Self;
    fn tanh(x: Self) -> Self;

    fn asinh(x: Self) -> Self;
    fn acosh(x: Self) -> Self;
    fn atanh(x: Self) -> Self;

    fn exp(x: Self) -> Self;
    fn exp2(x: Self) -> Self;

    fn ln(x: Self) -> Self;
    fn log(x: Self, base: Self) -> Self;
    fn log2(x: Self) -> Self;
    fn log10(x: Self) -> Self;

    fn pi() -> Self;
    fn two_pi() -> Self;
    fn frac_pi_2() -> Self;
    fn frac_pi_3() -> Self;
    fn frac_pi_4() -> Self;
    fn frac_pi_6() -> Self;
    fn frac_pi_8() -> Self;
    fn frac_1_pi() -> Self;
    fn frac_2_pi() -> Self;
    fn frac_2_sqrtpi() -> Self;
    fn sqrt2() -> Self;
    fn frac_1_sqrt2() -> Self;
    fn e() -> Self;
    fn log2_e() -> Self;
    fn log10_e() -> Self;
    fn ln_2() -> Self;
    fn ln_10() -> Self;

    fn to_degrees(x: Self) -> Self;
    fn to_radians(x: Self) -> Self;

    fn nan() -> Self;
    fn infinity() -> Self;
    fn neg_infinity() -> Self;
    fn neg_zero() -> Self;

    fn is_nan(self) -> bool;
    fn is_infinite(self) -> bool;
    fn is_finite(self) -> bool;
    fn is_normal(self) -> bool;
    fn classify(self) -> FPCategory;

    fn mantissa_digits(_: Option<Self>) -> uint;
    fn digits(_: Option<Self>) -> uint;
    fn epsilon() -> Self;
    fn min_exp(_: Option<Self>) -> int;
    fn max_exp(_: Option<Self>) -> int;
    fn min_10_exp(_: Option<Self>) -> int;
    fn max_10_exp(_: Option<Self>) -> int;

    fn ldexp(x: Self, exp: int) -> Self;
    fn frexp(x: Self) -> (Self, int);

    fn exp_m1(x: Self) -> Self;
    fn ln_1p(x: Self) -> Self;
    fn mul_add(x: Self, a: Self, b: Self) -> Self;
    fn next_after(x: Self, other: Self) -> Self;
}

#[inline] fn floor<T:Float>(x: T) -> T { Float::floor(x) }
#[inline] fn ceil<T:Float>(x: T) -> T { Float::ceil(x) }
#[inline] fn round<T:Float>(x: T) -> T { Float::round(x) }
#[inline] fn trunc<T:Float>(x: T) -> T { Float::trunc(x) }
#[inline] fn fract<T:Float>(x: T) -> T { Float::fract(x) }

#[inline] fn recip<T:Float>(x: T) -> T { Float::recip(x) }

#[inline] fn pow<T:Float>(x: T, n: T) -> T { Float::pow(x, n) }
#[inline] fn sqrt<T:Float>(x: T) -> T { Float::sqrt(x) }
#[inline] fn rsqrt<T:Float>(x: T) -> T { Float::rsqrt(x) }
#[inline] fn cbrt<T:Float>(x: T) -> T { Float::cbrt(x) }
#[inline] fn hypot<T:Float>(x: T, y: T) -> T { Float::hypot(x, y) }

#[inline] fn sin<T:Float>(x: T) -> T { Float::sin(x) }
#[inline] fn cos<T:Float>(x: T) -> T { Float::cos(x) }
#[inline] fn tan<T:Float>(x: T) -> T { Float::tan(x) }

// ...

This is related to the numeric trait reform tracked at #4819

@brendanzab
Copy link
Member Author

Another idea would be to remove generic maths completely from std and leave it up to to client libs to implement. I'm not sure how prevalent usage of generic maths is in std however. I realise that would be a complete reversal of my work in #4819 however.

@brendanzab
Copy link
Member Author

Of course some traits would still be left - I'm mainly talking about the math functions.

@brson
Copy link
Contributor

brson commented Jan 8, 2014

Nominating.

@brendanzab
Copy link
Member Author

I'm actually starting to think we should just zap the traits entirely (yay, back to the beginning!), and bring on the fragmentation! We still don't know how best to use traits yet, so maybe it would be better to leave that up to the community.

brendanzab added a commit to brendanzab/rust that referenced this issue Jan 9, 2014
The methods contained in `std::num::{Algebraic, Trigonometric, Exponential, Hyperbolic}` have now been moved into `std::num::Real`. This is part of an ongoing effort to simplify `std::num` (see issue rust-lang#10387).

`std::num::RealExt` has also been removed from the prelude because it is not a commonly used trait.
bors added a commit that referenced this issue Jan 9, 2014
The methods contained in `std::num::{Algebraic, Trigonometric, Exponential, Hyperbolic}` have now been moved into `std::num::Real`. This is part of an ongoing effort to simplify `std::num` (see issue #10387).

`std::num::RealExt` has also been removed from the prelude because it is not a commonly used trait.

r? @alexcrichton
@pnkfelix
Copy link
Member

pnkfelix commented Jan 9, 2014

Accepted for 1.0, P-backcompat-libs

@brendanzab
Copy link
Member Author

I'm now working on what I hope to become something like extra::num in this repository: https://github.com/bjz/num-rs

What I'm planning is to do is make std::num very naive and tied to the base numeric types, but then have extra::num founded in reasonably good mathematical concepts. That would avoid the ugly compromise we've made in the current std::num.

How does this sound?

@emberian
Copy link
Member

👍

@cartazio
Copy link
Contributor

oo, i like this stuff. prevent lockin while making is sane to experiment. 👍

bors added a commit that referenced this issue Jan 16, 2014
One less trait in `std::num` and three less exported in the prelude.

cc. #10387
bors added a commit that referenced this issue Jan 18, 2014
The patch adds the missing pow method for all the implementations of the
Integer trait. This is a small addition that will most likely be
improved by the work happening in #10387.

Fixes #11499
bors added a commit that referenced this issue Jan 18, 2014
As part of #10387, this removes the `Primitive::{bits, bytes, is_signed}` methods and removes the trait's operator trait constraints for the reasons outlined below:

- The `Primitive::{bits, bytes}` associated functions were originally added to reflect the existing `BITS` and `BYTES`statics included in the numeric modules. These statics are only exist as a workaround for Rust's lack of CTFE, and should be deprecated in the future in favor of using the `std::mem::size_of` function (see #11621).

- `Primitive::is_signed` seems to be of little utility and does not seem to be used anywhere in the Rust compiler or libraries. It is also rather ugly to call due to the `Option<Self>` workaround for #8888.

- The operator trait constraints are already covered by the `Num` trait.
brendanzab added a commit to brendanzab/rust that referenced this issue Feb 16, 2014
This is part of the effort to simplify `std::num`, as tracked in issue rust-lang#10387.
bors added a commit that referenced this issue Feb 18, 2014
This is part of the effort to simplify `std::num`, as tracked in issue #10387. It is also a step towards a proper IEEE-754 trait (see #12281).
bors added a commit that referenced this issue Feb 21, 2014
This is part of the effort to simplify `std::num`, as tracked in issue #10387.
@brson
Copy link
Contributor

brson commented Apr 15, 2014

@bjz What's left on this?

@brendanzab
Copy link
Member Author

We could merge Round into Float and Bitwise into Int (possibly renaming to Integer).

brendanzab added a commit to brendanzab/rust that referenced this issue Apr 19, 2014
Move the rounding functions into the `std::num::Float` trait and then remove `std::num::Round`.

This continues the flattening of the numeric traits tracked in rust-lang#10387. The aim is to make `std::num` very simple and tied to the built in types, leaving the definition of more complex numeric towers to third-party libraries.

[breaking-change]
bors added a commit that referenced this issue Apr 23, 2014
This pull request:

- Merges the `Round` trait into the `Float` trait, continuing issue #10387.
- Has floating point functions take their parameters by value.
- Cleans up the formatting and organisation in the definition and implementations of the `Float` trait.

More information on the breaking changes can be found in the commit messages.
@brson
Copy link
Contributor

brson commented Jun 17, 2014

@bjz What's left on this now?

@aturon
Copy link
Member

aturon commented Sep 16, 2014

As a heads-up: as part of library stabilization, I am now working on removing all the numerics traits (as per @bjz's comment above. These traits are currently providing little value in std, and we're better served by a library of traits and implementations in the Cargo ecosystem.

@cartazio
Copy link
Contributor

👍

@aturon
Copy link
Member

aturon commented Sep 16, 2014

After some discussion with @alexcrichton, it looks like this is probably an RFC-worthy change. I will write an RFC in the near future and link to it from here.

@brendanzab
Copy link
Member Author

Nice! Are there going to be any issues in providing polymorphic functions in the bundled libraries though?

@aturon
Copy link
Member

aturon commented Sep 17, 2014

@bjz The RFC will likely keep Int and Float traits. Methods on primitive int and float types have to be added via extension traits anyway, and since the API is identical across various sizes having a single trait makes sense. I'm still working out the precise details, though.

@brendanzab
Copy link
Member Author

Can we remove them from the prelude? looks hopefully

@aturon
Copy link
Member

aturon commented Sep 17, 2014

If we don't have them in the prelude, the various int/float methods they define won't be available by default (you'd have to import the traits). But these are pretty specialized methods, so that might be OK.

What's are the drawbacks you see to having these in the prelude?

@brendanzab
Copy link
Member Author

It will handicap future experimentation with numeric APIs by polluting the method namespace. It would be a shame for folks to have to go down the route of the numeric-prelude in the future.

Perhaps it would be cool if we could have:

// num.rs

pub use Int::{count_ones, leading_zeros, ... };

pub trait Int: ... {
    fn count_ones(self) -> uint;
    fn leading_zeros(self) -> uint;
    ...
}

@aturon
Copy link
Member

aturon commented Oct 8, 2014

RFC here: rust-lang/rfcs#369

@mdinger
Copy link
Contributor

mdinger commented Oct 19, 2014

I'm not sure if you're keeping so many pi constants around...I looked but couldn't tell. If they're staying, I thought they'd be easier to comprehend if written like this:

// Replace these
fn pi() -> Self;
fn two_pi() -> Self;
fn frac_pi_2() -> Self;
fn frac_2_sqrtpi() -> Self;

// With these:
fn pi() -> Self;
fn two_pi() -> Self;
fn pi_over_2() -> Self;
fn 2_over_sqrtpi() -> Self;

@aturon
Copy link
Member

aturon commented Nov 16, 2014

@bjz I think we can close this now, agreed?

@brendanzab
Copy link
Member Author

Closed :)

aturon referenced this issue in aturon/rust Nov 19, 2014
flip1995 pushed a commit to flip1995/rust that referenced this issue Mar 10, 2023
Fix rust-lang#107877, etc.

Fix rust-lang#10009
Fix rust-lang#10387
Fix rust-lang#107877

The fix is to verify that the associated item's trait is implemented before trying to project the item's type.

r? `@Jarcho`

---

changelog: ICE: [`needless_borrow`]: No longer panics on ambiguous projections
[rust-lang#10403](rust-lang/rust-clippy#10403)
<!-- changelog_checked -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system
Projects
None yet
Development

No branches or pull requests

9 participants