the trait IntoIterator is not implemented for ndarray::Zip #1182

Activity

1. bluss commented on Jul 6, 2022

   

   bluss

   on Jul 6, 2022

   Member

   More actions

   Zip is not an iterator so traversal needs to be done using its for_each method or similar methods - this is by design, it cannot give good performance in a for loop.

   Indexes are tuples because when this feature was designed let (i, j) = (1, 2) was valid in Rust while let [i, j] = [1, 2] was not - nowadays both are, so we could have picked arrays at this point.
2. bluss

   closed this as completedon Jul 6, 2022
3. bluss

   added

   question

   on Jul 6, 2022
4. jturner314 commented on Jul 7, 2022

   

   jturner314

   on Jul 7, 2022

   Last edited by jturner314

   Member

   More actions

   Fwiw, here are a couple of ways using Zip's existing methods to do a filter and sum:

   use ndarray::prelude::*;
   use ndarray::Zip;
   
   fn main() {
       let voxels = Array3::zeros([3, 4, 5]);
   
       // Using `for_each`.
       let mut sum1 = 0.;
       Zip::indexed(&voxels).for_each(|position, &density| {
           if ... {
               sum1 += ...;
           }
       });
   
       // Using `fold`.
       let sum2 = Zip::indexed(&voxels).fold(0., |acc, position, &density| {
           if ... {
               acc + ...
           } else {
               acc
           }
       });
   }
5. jimy-byerley commented on Jul 7, 2022

   

   jimy-byerley

   on Jul 7, 2022

   Last edited by jimy-byerley

   Author

   More actions

   this is by design, it cannot give good performance in a for loop

   Why wouldn't it give good performance ? I mean the current Zip::for_each is already doing kind of a sequential iteration right ?
   Also what's the point in it being a Parallelterator but not a SequentialIterator ? I mean for the design consistence, because what can be parallelized can easily be sequential (that's just like parallel but with only 1 thread in total)

   nowadays both are, so we could have picked arrays at this point.

   Do you think this could change in the future ? When I'm working with algorithms generic to dimensions, I often need to do things like

   Zip::indexed(&array).for_each(|position: [usize;N], field|  {
       Vector::from(position) + offset   // using the index position as a vector
       matrix_transform * Vector::from(position)   // using the index position as a point in space
       position[selected_dimension] += 1   // selecting dimensions by index
       // etc
   })

   and instead I have to specialize the implementation for each dimension, extracting the index coordinates one by one from the tuple

   Zip::indexed(&array).for_each(|position: (usize,usize,usize), field|  {   // specific to dimension 3
       Vector::new(position.0, position.1, position.2) + offset   // this is specific to array of dimension 3
       matrix_transform * Vector::newposition.0, position.1, position.2)   // specific AND verbose
       // much more verbose when working indices
       if selected_dimension == 0  {position.0 += 1;}
       else if  selected_dimension == 1  {position.1 += 1;}
       else if  selected_dimension == 2  {position.2 += 1;}
       else  { panic!("unsupported dimension"); }
       // etc
   })

   here are a couple of ways using Zip's existing methods to do a filter and sum

   That's what I ended up with ;) but I find it less ... idiomatic rust. Also I cannot combine Zip with the methods provided by random traits from other crates ...
6. rust-ndarray

   locked and limited conversation to collaborators on Jul 7, 2022
7. bluss

   converted this issue into a discussion #1184 on Jul 7, 2022