num-traits icon indicating copy to clipboard operation
num-traits copied to clipboard

Add 'absolute value of difference' utility function?

Open alercah opened this issue 4 years ago • 6 comments

I recently found myself wanting "absolute value of the difference" for potentially-unsigned values, thought that abs_sub might be it, found that it wasn't implemented for Unsigned, and then found out it wasn't what I wanted.

This is a useful-enough operation, and one that's annoying to write for unsigned values, since you either have to do match x.saturating_sub(y) == 0 { 0 => y.saturating_sub(x), v => v } or if x <= y { y - x } else { x - y }. Could this be added to num_traits, ideally to Num with one of the above two implementations used as a default, and with specialized impls for unsigned types that can just use abs. I believe that this would be forward-compatible with a default implementation for all Signed types once specialization is available.

alercah avatar Dec 03 '19 18:12 alercah

I don't think we have the appropriate super-traits to be able to add this fully generically. Simple comparison needs PartialOrd at least. We don't yet have any traits for saturating math at all, and even if we add that, it would be a breaking change to add a requirement to Num or Unsigned.

I think we could add it to PrimInt though, as it has PartialOrd. We could also add it as a new trait and/or a new bare function with appropriate constraints.

One designed consideration for signed integers -- should they return Self which can easily overflow, or return an unsigned type instead? e.g. consider abs_diff(-100i8, 100i8)

cuviper avatar Dec 03 '19 19:12 cuviper

Hm, that's a good point.

Since you can already call abs directly on signed types, the need for it is a bit weaker there. But I hadn't thought about the need for another bound on Unsigned (you do have the Saturating trait, incidentally, but #40 is a thing).

It could be implemented as a function that relied on one or the other of the traits to work, just without the ability to specialize. I just realized though that the Saturating approach would actually require Clone as well, so I think that PartialOrd is the strong preference here. In most cases, it's also probably at worst as fast as the saturating version, and at best might be significantly faster (e.g. for bignum types).

If specialization is desired, I guess the only reasonable approach would be a standalone trait + helper function.

For signed types, the ability to convert to unsigned might be useful, but in my mind that runs up into questions like #64 and whether there should generally be an "unsigned version of this signed integer" type which can also do overflowing abs with a little less pain than the built-in. We could possibly do that by doing something like:

pub trait PromoteUnsigned {
    type Promoted;
    fn abs_difference(self, other: Self) -> Self::Dist;
    fn promoting_abs(self) -> Self::Promoted;
}

Then there could be a blanket impl for PartialOrd + Unsigned. Or perhaps abs_difference should be in a separate trait...

Aside: the version number seems possibly off given the concerns about backwards-compatibility. ;)

alercah avatar Dec 03 '19 20:12 alercah

Thought: it might be viable by putting a PartialOrd bound directly on the method. Since there would be a default implementation provided, I don't think this would be a breaking change, and it would compile fine for types without the constraint and would just be uncallable.

trait Unsigned {
    ...
    fn abs_difference(self, other: Self) -> Self where Self: PartialOrd;
}

alercah avatar Dec 03 '19 20:12 alercah

Thought: it might be viable by putting a PartialOrd bound directly on the method.

Oh, yes! We already did similar for One::is_one too.

cuviper avatar Dec 03 '19 21:12 cuviper

Is it not good enough to just add a plain ol' function rather than a method?

fn abs_difference<T: Num + PartialOrd>(a: T, b: T) -> T {
    if a <= b {
        b - a
    } else {
        a - b
    }
}

That's what I'm using now, and it seems like adding it to the crate is pretty easy?

BartMassey avatar Mar 24 '20 16:03 BartMassey

The standard library now includes .abs_diff() on primitive types (see here for example). Would it make sense to add this as a trait now?

samueltardieu avatar Dec 12 '22 11:12 samueltardieu