iliekturtles / uom

Units of measurement -- type-safe zero-cost dimensional analysis
Apache License 2.0
1.02k stars 97 forks source link

How to get Ord and Eq with f64 underlying storage type? #208

Open rsivek opened 4 years ago

rsivek commented 4 years ago

Please bear with me, I'm still getting familiar with Rust. I'd really like to use this library to handle unit conversions but I ran into an issue with my need to have a sorted BTreeMap of Time keys.

use std::collections::BTreeMap;
use uom::si::f64::Time;

pub struct Something {
    my_map: BTreeMap<Time, i32> 
}

impl Something {
    pub fn new() -> Something {
        Something {
            my_map: BTreeMap::new()
        }
    }
}

This gave me the following error:

the trait bound `f64: std::cmp::Ord` is not satisfied

the trait `std::cmp::Ord` is not implemented for `f64`

note: required because of the requirements on the impl of `std::cmp::Ord` for `uom::si::Quantity<dyn uom::si::Dimension<Kind = (dyn uom::Kind + 'static), Th = uom::typenum::Z0, I = uom::typenum::Z0, M = uom::typenum::Z0, L = uom::typenum::Z0, N = uom::typenum::Z0, T = uom::typenum::PInt<uom::typenum::UInt<uom::typenum::UTerm, uom::typenum::B1>>, J = uom::typenum::Z0>, dyn uom::si::Units<f64, amount_of_substance = uom::si::amount_of_substance::mole, length = uom::si::length::meter, luminous_intensity = uom::si::luminous_intensity::candela, mass = uom::si::mass::kilogram, electric_current = uom::si::electric_current::ampere, thermodynamic_temperature = uom::si::thermodynamic_temperature::kelvin, time = uom::si::time::second>, f64>`
note: required by `std::collections::BTreeMap::<K, V>::new`

This makes sense because Ord isn't implemented for native float types, and Ord is necessary for keeping things sorted in the BTreeMap. So I reached for a wrapper which would take care of that. The ordered-float crate looked like a popular choice so I went with that.

use std::collections::BTreeMap;
use uom::si::f64::Time;
use ordered_float::OrderedFloat

pub struct Something {
    my_map: BTreeMap<OrderedFloat<Time>, i32> 
}

impl Something {
    pub fn new() -> Something {
        Something {
            my_map: BTreeMap::new()
        }
    }
}

That gives me a new error:

the trait bound `uom::si::Quantity<dyn uom::si::Dimension<Kind = (dyn uom::Kind + 'static), Th = uom::typenum::Z0, I = uom::typenum::Z0, M = uom::typenum::Z0, L = uom::typenum::Z0, N = uom::typenum::Z0, T = uom::typenum::PInt<uom::typenum::UInt<uom::typenum::UTerm, uom::typenum::B1>>, J = uom::typenum::Z0>, dyn uom::si::Units<f64, amount_of_substance = uom::si::amount_of_substance::mole, length = uom::si::length::meter, luminous_intensity = uom::si::luminous_intensity::candela, mass = uom::si::mass::kilogram, electric_current = uom::si::electric_current::ampere, thermodynamic_temperature = uom::si::thermodynamic_temperature::kelvin, time = uom::si::time::second>, f64>: uom::num::Float` is not satisfied

the trait `uom::num::Float` is not implemented for `uom::si::Quantity<dyn uom::si::Dimension<Kind = (dyn uom::Kind + 'static), Th = uom::typenum::Z0, I = uom::typenum::Z0, M = uom::typenum::Z0, L = uom::typenum::Z0, N = uom::typenum::Z0, T = uom::typenum::PInt<uom::typenum::UInt<uom::typenum::UTerm, uom::typenum::B1>>, J = uom::typenum::Z0>, dyn uom::si::Units<f64, amount_of_substance = uom::si::amount_of_substance::mole, length = uom::si::length::meter, luminous_intensity = uom::si::luminous_intensity::candela, mass = uom::si::mass::kilogram, electric_current = uom::si::electric_current::ampere, thermodynamic_temperature = uom::si::thermodynamic_temperature::kelvin, time = uom::si::time::second>, f64>`

note: required because of the requirements on the impl of `std::cmp::Ord` for `ordered_float::OrderedFloat<uom::si::Quantity<dyn uom::si::Dimension<Kind = (dyn uom::Kind + 'static), Th = uom::typenum::Z0, I = uom::typenum::Z0, M = uom::typenum::Z0, L = uom::typenum::Z0, N = uom::typenum::Z0, T = uom::typenum::PInt<uom::typenum::UInt<uom::typenum::UTerm, uom::typenum::B1>>, J = uom::typenum::Z0>, dyn uom::si::Units<f64, amount_of_substance = uom::si::amount_of_substance::mole, length = uom::si::length::meter, luminous_intensity = uom::si::luminous_intensity::candela, mass = uom::si::mass::kilogram, electric_current = uom::si::electric_current::ampere, thermodynamic_temperature = uom::si::thermodynamic_temperature::kelvin, time = uom::si::time::second>, f64>>`
note: required by `std::collections::BTreeMap::<K, V>::new`

Now, this error I don't quite understand. Apparently the Float trait is not satisfied even though the underlying type is f64? Why would that be?

Do you have any suggestions for allowing these types to be used with sorted collections such as BTreeMap?

adamreichold commented 4 years ago

Quantity cannot implement Float as methods like

fn powi(self, n: i32) -> Self

are not applicable as they would change the dimension. (Have a look at Quantity::powi to compare the signatures.)

I suppose the easiest way for you to combine these is to create a newtype wrapper, e.g.

#[derive(PartialOrd)]
struct OrderedTime(Time);

impl Ord for OrderedTime {
  fn cmp(&self, other: &Self) -> Ordering {
    self.partial_cmp(other).unwrap()
  }
}

which corresponds roughly to ordered_float::NotNan but you could also implement cmp via

OrderedFloat(self.value).cmp(&OrderedFloat(other.value))
rsivek commented 4 years ago

Understood. Thank you for your quick response, this is very helpful!

iliekturtles commented 4 years ago

Thanks for the quick response @adamreichold!

Similar to how the standard library doesn't impl Ord for floating point primitives I think it should be out of scope or uom as well. If there are any other crates that impl Ord for floating point types that have lesser requirements than fully implementing Float that would a possibility to support.

Any other questions, @nanowizard, or are we OK to close this issue?

rsivek commented 4 years ago

@iliekturtles I agree its somewhat out of scope for uom. At the same time a wrapper library that implements Ord for f32 or f64 based Quantity types would be quite useful so we didn't have to roll our own. I haven't done an exhaustive search to see if there are any crates that provide Ord for floats that don't fully implement Float, but if a good one exists I would suggest maybe putting a reference to it in the docs.

Lucretiel commented 4 years ago

ordered_float is definitely the standard for this. It provides two types: NotNan and OrderedFloat, both of which define a total ordering. The only thing it needs to work today is an implementation of uom::Conversion.

iliekturtles commented 4 years ago

Re-opening to investigate supporting ordered_float behind a feature gate. See #216 for considerations about the feature name.

douweschulte commented 9 months ago

For some code I am writing I needed the ordered units. I initially wrote some wrappers around the quantity I use most to be able to use that totally ordered. But then found OrderedFloat and tried to get uom to work with these types. After a couple of hours of playing around I could not get it to work in uom itself. I assume I do not understand well enough how uom actually works under the hood, but I am very interested in this feature. If there is someone that can guide me a bit I would be happy to put in some more work. _If there is any interest my baby steps can be found here: https://github.com/douweschulte/uom/tree/ordered_float_208_