rust-num / num-traits

Numeric traits for generic mathematics in Rust
Apache License 2.0
732 stars 135 forks source link

No `.abs()` on integers, any reason? #64

Open amandasaurus opened 6 years ago

amandasaurus commented 6 years ago

There are some method on Float which aren't on any of the integer traits (PrimInt? Num Itself). e.g. .abs(). A i32 can defintely have an .abs() method. (.abs() on a u8 makes sense (ish)). Something I'm doing would benefit from more generic numeric traits.

Is there any reason why Float has methods which could be applied to more generic numbers? I don't want to submit a patch if there's some good reason I'm not aware of

cuviper commented 6 years ago

There is Signed::abs(). You're right that abs() (and probably all of Signed) could be trivially implemented for unsigned numbers too, but the trait is somewhat useful as a marker too.

maxbla commented 5 years ago

Would it make sense to split up the marker trait from operations involving sign? There could be a trait called SignOps or something like that, which contains all methods of Signed as it stands now. Then repurpose the Signed trait as a marker for signed vs unsigned types.

cuviper commented 5 years ago

We also tend to stick to methods that are inherently available on the types. (I won't claim this universally, as I'm sure there are exceptions, but just as a general tendency.) So on that note, the unsigned primitive integers don't provide abs() or any of the other Signed methods that would be const/no-op.

I'm wary of adding that, because it seems like a code-smell if you're doing signed-like operations on something unsigned. At the least, I'd like to see a real motivating case for this.

maxbla commented 5 years ago

Off the top of my head, I was just doing some work on formatting traits for Ratio<T> in the rust-rational crate. I wanted some way to tell if the numerator was negative, so I could add a - character. To do that, I settled on the following hack (for now)

let non_negative = !(self < &<Ratio<T> as Zero>::zero());

If all Integers (including unsigned ones) implemented SignOps with some sort of is_positive() or is_negative() functions, I wouldn't have to do that.

I think more generally that splitting up the traits would be useful for an algorithm that has to deal with sign explicitly, but that is generic across both signed and unsigned Integers.

I agree that abs() on an unsigned number is smelly, but I'm mostly after the is_positive and is_negative functions.

cuviper commented 5 years ago

OK, well... adding a new SignOps is no issue if we decide it is needed. Grafting it into the current trait hierarchy is a breaking change though.

maxbla commented 5 years ago

We could make this not a breaking change by having a SignOps trait that has a blanket implementation for Signed

cuviper commented 5 years ago

But you can't make it a supertrait of Integer without addressing the unsigned types too.

maxbla commented 5 years ago

Am I wrong to assume that you could have a blanket implementation for Signed types and bespoke (non-blanket?) implementations for all the unsigned Integers?

cuviper commented 5 years ago

I don't think that's possible without specialization.

thehappycheese commented 2 years ago

I don't fully understand the discussion above... I can only contribute by providing an example of why this feels like a problem and how google brought me here;

I am attempting to contribute to a library (geo) which is forcing me to work with a type (CoordFloat) constrained to num_traits::Float but not constrained to num_traits::Signed (I'm surprised but assuming there is a good reason that Floats aren't always Signed)

But now I want to use num_traits::abs() and I am getting an error. I am looking for a neat work around for this problem;

fn cross_product<T>(left: Coord<T>, right: Coord<T>) -> T
where
  T: CoordFloat,
{
  left.x * right.y - left.y * right.x
}

// ...

fn offset<T>(ls:LineString<T>, distance: T) -> LineString<T>
where 
  T : CoordFloat // Constrained to num_traits::Float
{
  // ...
  let ab:Coord<T> = *b - *a;
  let cd:Coord<T> = *d - *c;
  let my_value = cross_product(ab, cd);
  if num__traits::abs(my_value) < num_traits::cast(0.0000001f64).unwrap() { 
    //                ^^^^^^^^ doesn't work
    // ...
  }
  // ...
}

I can see a few undesirable solutions;

Many thanks

cuviper commented 2 years ago

@thehappycheese can't you just use Float::abs? You even get method syntax, my_value.abs().

thehappycheese commented 2 years ago

@thehappycheese can't you just use Float::abs? You even get method syntax, my_value.abs().

Hi @cuviper, thanks for the response. Ah apologies yes that does work. I am not sure how I did not think to try that... Now I found 3 ways to do it, 2 of them work, 1 looks like it should but doesn't:

pub(super) fn line_intersection_with_parameter<T>(
    a: &Coord<T>,
    b: &Coord<T>,
    c: &Coord<T>,
    d: &Coord<T>,
) -> LineIntersectionWithParameterResult<T>
where
    T: CoordFloat,  // CoordFloat is constrained to num_traits Float but not Signed
{
    //    ...
    let ab_cross_cd:T = cross_product(ab, cd);

    // Works
    let test = T::abs(ab_cross_cd);
    // Works
    let test = ab_cross_cd.abs();
    // Does not work:
    let test = num_traits::abs(ab_cross_cd); // T is not Signed
    // ...
}