the trait IntoIterator is not implemented for ndarray::Zip

[jimy-byerley]

Hello dear maintainers, and thank you for your work on this crate !
I'm currently writing some voxel-processing functions and I'm facing a strange fact: the Zip iterator-like struct of ndarray doesn't seems to implement the IntoIterator trait. It does implement however the IntoParallelIterator trait, but it is of no help in my case where I want to filter and sum over the iterator elements.

The probleme of not implementing the Iterator trait is a general concern I think, but I'm specifically trying to do something like this:

// idiomatic rust
let acc = Zip::indexed(voxel)
                .filter_map(|position, density|  {...})
                .sum()
// imperative style
let mut acc = 0.;
for (position, density) in Zip::indexed(voxel) {
    if ...  { acc += ... }
}

None of the above ways are working because of the IntoIterator trait missing. I also tried ArrayBase::indexed_iter() but it gives tuple indices instead of fixed-arrays (not convenient for further compuations); plus it iterates in the logical order so not the most efficient depending on the input strides.

Could possibily you implement IntoIterator for Zip as it already have IntoParallelIterator ?

Edit:

in fact, both Zip::indexed and ArrayBase::indexed_iter yield a tuple for index. not sure why because the docs says I should get a [usize; 3] instead. I would be much more convenient to have such an array in my opinion ... but this is an other question.

[bluss]

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.

[jturner314]

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
        }
    });
}

[jimy-byerley]

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

[bluss]

It is unfortunately better for ndarray to not provide an iterator implementation. Users could experience a 10x slowdown or worse, it's just not good to leave the alternative open when the cost is so high. The library is helping the user by making the path to performance easier. (With that said, of course ndarray's performance story is not perfect.)

The general performance problem we are facing with pull-based iteration is described here https://medium.com/@veedrac/rust-is-slow-and-i-am-the-cure-32facc0fdcb It's a problem that, in a simplified sense, occurs when you have more than one axis of iteration. Even VecDeque in std has this problem (it has two "axes" internally: which linear index it is visiting, and which half of the deque it is visiting).

[jimy-byerley]

Very interesting reading. And it seems no trait Allwas ever introduced in the std :-/