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

using .quantile_mut() on Array1::<f64> #58

Open
CharlieBickerton opened this issue Dec 3, 2019 · 6 comments
Open

using .quantile_mut() on Array1::<f64> #58

CharlieBickerton opened this issue Dec 3, 2019 · 6 comments
Labels
Enhancement New feature or request Good first issue Good for newcomers

Comments

@CharlieBickerton
Copy link

CharlieBickerton commented Dec 3, 2019

Hi guys, I'm trying to use .quantile_mut() on a 1D array of f64 numbers. I'm getting this error:

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

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

I have a few questions:

  1. Which type will work? It's not clear from the docs.
  2. Is there a reason for using the noisy float for the number between 0-1? If the function errors when it's not between 0-1, wouldnt a normal float work?
@LukeMathWalker
Copy link
Member

Rust's floats (f64 and f32) do not implement the Ord trait because of how f64::NAN and f32::NAN behave in comparisons - see this little snippet https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6190375f8d1a48bc1a885ce38981ebea

If you remove Not a Number from the equation (using something like noisy-floats) than you recover the Ord trait and you can call .quantile_mut.

As an action item on our side, we should probably provide a PartialOrd version of quantile_mut, as we do for the other summary function.

@LukeMathWalker LukeMathWalker added Enhancement New feature or request Good first issue Good for newcomers labels Dec 4, 2019
@CharlieBickerton
Copy link
Author

CharlieBickerton commented Dec 4, 2019

Thanks for the quick response @LukeMathWalker , that's annoying f32/f64 doen't impelment Ord . I made my array n64's and it works great.

It was potentially me reading the docs wrong as I had the impression that the q value needed to be noisy rather than the array which didn't make much sense to me. Thanks for clearning it up!

@jturner314
Copy link
Member

The quantile_axis_skipnan_mut method on the QuantileExt trait may also be useful for your application since it can work directly on f32/f64 in the presence of NaN values. We should probably add an analogous method to Quantile1dExt; @LukeMathWalker is that what you were referring to in your comment?

@CharlieBickerton
Copy link
Author

@jturner314 that sounds like a great idea, thanks for pointing out quantile_axis_skipnan_mut, I could add an axis to my 1d arrays and use this but I think it's cleaner to stay in 1d.

@LukeMathWalker
Copy link
Member

Yes, that's what I meant @jturner314.

@jturner314
Copy link
Member

@CharlieBickerton Note that 1d arrays have a single axis (axis 0). quantile_axis_skipnan_mut returns an array with one less axis, so the result of calling it on a 1d array is a 0d array. To turn an 0d array into its element, you can use .into_scalar(). So, you could do this:

use ndarray::prelude::*;
use ndarray_stats::{interpolate::Nearest, QuantileExt};
use noisy_float::types::n64;

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let mut arr: Array1<f64> = array![0., std::f64::NAN, 1., std::f64::NAN, 2., 3., 4.];
    assert_eq!(
        2.,
        arr.quantile_axis_skipnan_mut(Axis(0), n64(0.5), &Nearest)?
            .into_scalar()
    );
    Ok(())
}

It would be more convenient to have a .quantile_skipnan_mut() method in Quantile1dExt, as @LukeMathWalker pointed out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants