[Numpy-discussion] ANN: NumPy 1.7.0b2 release

[Nathaniel Smith]

On Tue, Sep 25, 2012 at 1:27 AM, Ondřej Čertík <ondrej.certik at gmail.com> wrote:
> On Mon, Sep 24, 2012 at 3:49 PM, Nathaniel Smith <njs at pobox.com> wrote:
>> On Mon, Sep 24, 2012 at 10:47 PM, Charles R Harris
>> <charlesr.harris at gmail.com> wrote:
>>>
>>>
>>> On Mon, Sep 24, 2012 at 2:25 PM, Frédéric Bastien <nouiz at nouiz.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> I tested this new beta on Theano and discovered an interface change
>>>> that was not there in the beta 1.
>>>>
>>>> New behavior:
>>>> numpy.ndindex().next()
>>>> (0,)
>>>>
>>>> Old behavior:
>>>> numpy.ndindex().next()
>>>> ()
>>>>
>>>> This break some Theano code that look like this:
>>>>
>>>> import numpy
>>>> shape=()
>>>> out_shape=[12]
>>>> random_state=numpy.random.RandomState()
>>>>
>>>> out = numpy.zeros(out_shape, int)
>>>> for i in numpy.ndindex(*shape):
>>>>     out[i] = random_state.permutation(5)
>>>>
>>>>
>>>> I suppose this is an regression as the only mention of ndindex in the
>>>> first email of this change is that it is faster.
>>>>
>>>
>>> I think this problem has been brought up on the list. It is interesting that
>>> it turned up after the first beta. Could you do a bisection to discover
>>> which commit is responsible?
>>
>> No need, the problem is already known. It was introduced by that
>> ndindex speed up patch, PR #393, which was backported into the first
>> beta as well. There's a follow-up patch in PR #445 that fixes both of
>> these issues, though it also exposes some more fundamental issues with
>> the nditer API, so there's lots of discussion there about if we want
>> some more changes... this is a good summary:
>> https://github.com/numpy/numpy/pull/445#issuecomment-8740982
>>
>> For 1.7 purposes though the bottom line is that we already have
>> multiple acceptable solutions, so both the issues reported here should
>> definitely be fixed.
>
> Should we just remove (revert) this PR #393 patch from the release branch?
> It shouldn't have been there in the first place, the only reason I included it
> is because other patches depended on it and I would have to fix collisions,
> and we thought it would be harmless to just include it. Which turned out
> to be a mistake, for which I apologize.
>
> That way we'll feel confident that the branch works, and we can get the right
> solution into master and test it there.
>
> So I am actually convinced I should simply revert this patch in the
> release branch.
> Let me know what you think.

Sounds good to me. (I also thought it would be harmless to include it,
and also missed that the other patch that depended on it was part of
the same change and could be reverted too.)

-n