<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>since ndrange is a superset of the features of ndindex, we can implement ndindex with ndrange or keep it as is.</div><div>ndindex is now a glorified `nditer` object anyway. So it isn't so much of a maintenance burden. <br></div><div>As for how ndindex is implemented, I'm a little worried about python 2 performance seeing as range is a list. <br></div><div>I would wait on changing the way ndindex is implemented for now.<br></div><div><br></div><div>I agree with Stephan that ndindex should be kept in. Many want backward compatible code. It would be hard for me to justify why a dependency should be bumped up to bleeding edge numpy just for a convenience iterator.</div><div><br></div><div>Honestly, I was really surprised to see such a speed difference, I thought it would have been closer.</div><div><br></div><div></div><div>Allan, I decided to run a few more benchmarks, the nditer just seems slow for single array access some reason. Maybe a bug?</div><div><br></div><div>```</div><div>import numpy as np<br>import itertools<br>a = np.ones((1000, 1000))<br><br>b = {}<br>for i in np.ndindex(a.shape):<br> b[i] = i<br></div><div><br>%%timeit</div><div># op_flag=('readonly',) doesn't change performance<br></div><div>for a_value in np.nditer(a):<br> pass<br>109 ms ± 921 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)<br><br>%%timeit<br>for i in itertools.product(range(1000), range(1000)):<br> a_value = a[i]<br>113 ms ± 1.72 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)<br><br>%%timeit<br>for i in itertools.product(range(1000), range(1000)):<br> c = b[i]<br>193 ms ± 3.89 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)</div><div><br>%%timeit<br>for a_value in a.flat:<br> pass<br>25.3 ms ± 278 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)</div><div><br></div><div>%%timeit<br>for k, v in b.items():<br> pass<br>19.9 ms ± 675 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)</div><div><br></div><div>%%timeit<br>for i in itertools.product(range(1000), range(1000)):<br> pass<br>28 ms ± 715 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)<br></div><div>```<br></div></div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 8, 2018 at 4:26 PM Stephan Hoyer <<a href="mailto:shoyer@gmail.com">shoyer@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I'm open to adding ndrange, and "soft-deprecating" ndindex (i.e., discouraging its use in our docs, but not actually deprecating it). Certainly ndrange seems like a small but meaningful improvement in the interface.<div><br></div><div>That said, I'm not convinced this is really worth the trouble. I think the nested loop is still pretty readable/clear, and there are few times when I've actually found ndindex() be useful.</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 8, 2018 at 12:35 PM Allan Haldane <<a href="mailto:allanhaldane@gmail.com" target="_blank">allanhaldane@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 10/8/18 12:21 PM, Mark Harfouche wrote:<br>
> 2. `ndindex` is an iterator itself. As proposed, `ndrange`, like<br>
> `range`, is not an iterator. Changing this behaviour would likely lead<br>
> to breaking code that uses that assumption. For example anybody using<br>
> introspection or code like:<br>
> <br>
> ```<br>
> indx = np.ndindex(5, 5)<br>
> next(indx) # Don't look at the (0, 0) coordinate<br>
> for i in indx:<br>
> print(i)<br>
> ```<br>
> would break if `ndindex` becomes "not an iterator"<br>
<br>
OK, I see now. Just like python3 has separate range and range_iterator<br>
types, where range is sliceable, we would have separate ndrange and<br>
ndindex types, where ndrange is sliceable. You're just copying the<br>
python3 api. That justifies it pretty well for me.<br>
<br>
I still think we shouldn't have two functions which do nearly the same<br>
thing. We should only have one, and get rid of the other. I see two ways<br>
forward:<br>
<br>
* replace ndindex by your ndrange code, so it is no longer an iter.<br>
This would require some deprecation cycles for the cases that break.<br>
* deprecate ndindex in favor of a new function ndrange. We would keep<br>
ndindex around for back-compatibility, with a dep warning to use<br>
ndrange instead.<br>
<br>
Doing a code search on github, I can see that a lot of people's code<br>
would break if ndindex no longer was an iter. I also like the name<br>
ndrange for its allusion to python3's range behavior. That makes me lean<br>
towards the second option of a separate ndrange, with possible<br>
deprecation of ndindex.<br>
<br>
> itertools.product + range seems to be much faster than the current<br>
> implementation of ndindex<br>
> <br>
> (python 3.6)<br>
> ```<br>
> %%timeit<br>
> <br>
> for i in np.ndindex(100, 100):<br>
> pass<br>
> 3.94 ms ± 19.4 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)<br>
> <br>
> %%timeit<br>
> import itertools<br>
> for i in itertools.product(range(100), range(100)):<br>
> pass<br>
> 231 µs ± 1.09 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)<br>
> ```<br>
<br>
If the new code ends up faster than the old code, that's great, and<br>
further justification for using ndrange instead of ndindex. I had<br>
thought using nditer in the old code was fastest.<br>
<br>
So as far as I am concerned, I say go ahead with the PR the way you are<br>
doing it.<br>
<br>
Allan<br>
_______________________________________________<br>
NumPy-Discussion mailing list<br>
<a href="mailto:NumPy-Discussion@python.org" target="_blank">NumPy-Discussion@python.org</a><br>
<a href="https://mail.python.org/mailman/listinfo/numpy-discussion" rel="noreferrer" target="_blank">https://mail.python.org/mailman/listinfo/numpy-discussion</a><br>
</blockquote></div>
_______________________________________________<br>
NumPy-Discussion mailing list<br>
<a href="mailto:NumPy-Discussion@python.org" target="_blank">NumPy-Discussion@python.org</a><br>
<a href="https://mail.python.org/mailman/listinfo/numpy-discussion" rel="noreferrer" target="_blank">https://mail.python.org/mailman/listinfo/numpy-discussion</a><br>
</blockquote></div>