RStats - my first crates.io contribution

My Rust is still rusty, so please have a look and comment, I am always hoping to improve.
Apart from the technicalities of the language, I am trying to do useful and interesting things in RStats

You put this into the "code review" setion. Do you have code to be reviewed?

We can just follow the link from crate page to the repository. Is this link correct?

Thanks, yes, it is. You can also see the source from the Documentation link straight from the link I have given.
I would particularly like to know if I am neglecting some more "idiomatic" style of organising the traits etc.

The traits are a bit funky with how they consume self but are implemented on the reference types. I would probably have expected to see this:

pub trait MutVectors {
    fn mutsmult(&mut self, s: f64);
    fn mutvsub(&mut self, v: &[f64]);
    fn mutvadd(&mut self, v: &[f64]);
    fn mutvunit(&mut self);
}

impl MutVectors for [f64] {
   fn mutsmult(&mut self, s:f64) {
     self.iter_mut().for_each(|x|{ *x*=s });
   }
   fn mutvsub(&mut self, v: &[f64]) {
     self.iter_mut().enumerate().for_each(|(i,x)|*x-=v[i])
   }
   fn mutvadd(&mut self, v: &[f64]) {
     self.iter_mut().enumerate().for_each(|(i,x)|*x+=v[i])
   }
   fn mutvunit(&mut self) { 
      self.mutsmult(1_f64/self.iter().map(|x|x.powi(2)).sum::<f64>().sqrt())
   }
}
1 Like

Thank you @alice.

I had those '&' in there before but the users had to be forever calling .as_slice() (or .as_mut_slice for this particular trait) on the self Vecs. Because && does not match the default cast of Vec down to a slice. Sometimes these methods are applied to slices as well and having to keep track of which is which is an unnecessary complication for the users. Getting rid of the & simplified the use and it still works. Perhaps using && instead of & is, in fact, even funkier?

I guess my main question is this: is it possible, in this setup, to move to generics without actually making things a lot more complicated? In particular, so that the Vectors trait would work also on f32? In the process I would prefer to shorten the code rather than the opposite, though.

You would have to also implement the trait on the Vec<f64> type.

Hi i think that you don't need to enumerate to later index the slices, for example in the dot product could be better:

fn dot(v1: &[f32], v2: &[f32]) -> f32 {
    v1.iter().zip(v2).map(|(&a, &b)| a * b).sum()
}

one cool feature you could use is the windows() method:

one example could be a moving average function:

fn moving_average(slice: &[f32]) -> Vec<f32> {
    slice.windows(3).map(|element| (element[0] + element[1] + element[2]) / 3.0).collect()
}

fn main() {
    let v = [1.0, 20.0, 20.0, 10.0, 20.0, 4.0, 20.0, 10.0, 0.0, 20.0, 3.0, 5.0, 5.0];
    let mv = moving_average(&v);
    println!("mv: {:?}", mv);
}
1 Like

Looks like you don't need a pub on the impls modules, which will make your documentation easier to read.

1 Like

Alice: yes but I do not understand why I should duplicate all my code for no gain. Am I missing something?

In new version 0.5.0 I have now changed the multidimensional operations on sets of vectors from flat Vecs to Vec<Vec<f64>> type, after all, as it leads to better organized and safer code. It does not even seem to be any slower.