JErnestoMtz / rapl

Rank Polymorphic array library for Rust.
103 stars 3 forks source link

Map between types #6

Closed JErnestoMtz closed 1 year ago

JErnestoMtz commented 1 year ago

There is no easy way of converting Ndarr<T,R> into Ndarr<P,R>.

We should add a map that is generic over both T and P, that would allow:

let a_i32 = Ndarr::from([1,2,3]);
let a_f32 = a_i32.map(|x| x as f32)

Although maybe not call it differently, any suggestions are welcome.

S-Erik commented 1 year ago

In the ndarray crate, the map function is implemented as a method of the ArrayBase struct not as a trait as in this crate (rapl).

Additionally, the map function of the ndarray crate takes a reference of self instead of ownership as done in this crate (rapl).

What are the reasons for out map function to be a function of the Map trait, @JErnestoMtz?

Here is my current prototype implementation of a map function that can map between types, called map_types using a new trait MapTypes (note that we take a reference of self instead of taking ownership):


pub trait MapTypes<F> {
    type Output;

    fn map_types(&self, f: F) -> Self::Output;
}

impl<F, T1, T2, const R: usize> MapTypes<F> for Ndarr<T1, R>
where
    F: Fn(&T1) -> T2,
    T1: Debug + Clone + Default,
    T2: Debug + Clone + Default,
{
    type Output = Ndarr<T2, R>;

    fn map_types(&self, f: F) -> Self::Output {
        let mut out = vec![T2::default(); self.data.len()];
        for (i, val) in out.iter_mut().enumerate() {
            *val = f(&self.data[i])
        }
        Ndarr {
            data: out,
            shape: self.shape,
        }
    }
}

#[test]
fn map_between_types_test() {
    let x: Ndarr<i16, 2> = Ndarr::from([[1,3,4], [10, 42, 69]]);
    println!("x: {:#?}", x.data);
    println!("x shape: {:#?}", x.shape);
    let res: Ndarr<f32, 2> = map_between_types(x);
    println!("res: {:#?}", res);

    let a_i32: Ndarr<i32, 1> = Ndarr::from([1,2,3]);
    let a_f32: Ndarr<f32, 1> = a_i32.map_types(|x| *x as f32);
    let a_string: Ndarr<String, 1> = a_i32.map_types(|x| x.to_string());

    println!("a_i32: {:#?}", a_i32); // [1,2,3]
    println!("a_f32: {:#?}", a_f32); // [1.0,2.0,3.0]
    println!("a_string: {:#?}", a_string); // ["1","2","3"]
}

The map_types function takes a closure that takes as input something of type T1 and outputs something of type T2. As far as I know, this is not possible to incorporate into the map function of the Map trait, because then the Map trait has to have 2 generic types and verbose type annotations are needed every time we call the map function. This would be implemented something like this:

pub trait Map<F1, F2> {
    type OutType;

    fn map(self, f: F1) -> Self::OutType;

    fn map_in_place(&mut self, f: F2);
}

impl<F1, F2, T1, T2, const R: usize> Map<F1, F2> for Ndarr<T1, R>
where
    F1: Fn(&T1) -> T2,
    F2: Fn(&T1) -> T1,
    T1: Debug + Clone + Default,
    T2: Debug + Clone + Default,
{
    type OutType = Ndarr<T2, R>;

    fn map(self, f: F1) -> Self::OutType {
        let mut out = vec![T2::default(); self.data.len()];
        for (i, val) in out.iter_mut().enumerate() {
            *val = f(&self.data[i])
        }
        Ndarr {
            data: out,
            shape: self.shape,
        }
    }
    fn map_in_place(&mut self, f: F2) {
        for val in self.data.iter_mut() {
            *val = f(val)
        }
    }
}

I think, we do not need two traits (Map and MapTypes) if we implement map functions directly for the Ndarr struct instead of using traits.

Therefore, I suggest defining the map functions directly on the Ndarr struct instead of using traits (also using references instead of taking ownership), except there are other advantages for using the Map trait over using map functions.

JErnestoMtz commented 1 year ago

The main reason Map is a separate trait is mainly as a remnant of the development process.

The original idea was to use Ndarr and native Rust arrays almost interchangeably, for example:

let a = Ndarr::from([1,2,3])
let b = &a + [1,1,1];
assert_eq!(b, Ndarr::from([2,3,4]))

So it would be necessary for both Ndarr and native arrays to implement the same traits, including Map. Although this would be possible, I think it would add a lot of extra complexity and confusion. So I think you are correct in that we should transfer the map and map_in_place function directly into Ndarr.

JErnestoMtz commented 1 year ago

I might close this issue due to pull request: #9 if no further clarification is needed.

Thank a lot @S-Erik, I pretty much went for your implementation.