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

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

Open
rsivek opened this issue Oct 3, 2020 · 7 comments
Open

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

rsivek opened this issue Oct 3, 2020 · 7 comments

Comments

@rsivek
Copy link

rsivek commented Oct 3, 2020

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
Copy link
Contributor

adamreichold commented Oct 3, 2020

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
Copy link
Author

rsivek commented Oct 3, 2020

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

@iliekturtles
Copy link
Owner

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
Copy link
Author

rsivek commented Oct 6, 2020

@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.

@rsivek rsivek closed this as completed Oct 6, 2020
@Lucretiel
Copy link
Contributor

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
Copy link
Owner

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

@iliekturtles iliekturtles reopened this Nov 8, 2020
@douweschulte
Copy link

douweschulte commented Jan 23, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants