On Do, 2015-09-24 at 10:03 -0700, Nathaniel Smith wrote:
> On Sep 24, 2015 4:14 AM, "Sebastian Berg" <sebast...@sipsolutions.net>
> wrote:
> >
> > On Do, 2015-09-24 at 03:26 -0700, Stefan van der Walt wrote:
> > > On 2015-09-24 00:17:33, Jens Jørgen Mortensen <je...@fysik.dtu.dk>
> wrote:
> > > > jensj@jordan:~$ python
> > > > Python 2.7.9 (default, Apr  2 2015, 15:33:21)
> > > > [GCC 4.9.2] on linux2
> > > > Type "help", "copyright", "credits" or "license" for more
> information.
> > > >  >>> import numpy as np
> > > >  >>> a = np.zeros((2, 2, 2))
> > > >  >>> b = np.zeros((2, 2, 2))
> > > >  >>> a[0, 0] = 7
> > > >  >>> b[0, 0] = 6
> > > >  >>> np.vdot(a[:, :, 0], b[:, :, 0]).real
> > > > 84.0
> > > >  >>> np.__version__
> > > > '1.10.0rc1'
> > > >
> > > > The result should be 42.  This is on Ubuntu 15.04.
> > >
> > > The input is not supposed to be matrices—but the docstring
> specifically
> > > states that those should then be ravelled, so this is indeed a
> bug.
> > >
> > > If I bisected correctly, the problematic change is this one:
> > >
> >
> > Hmmm, unless we want to make sure that the output of ravel is always
> > contiguous (which would be a difference from `.reshape(-1)`.
> > It would be a bit safer, and not a useless feature maybe, since we
> can
> > point to `.reshape(-1)` as well for those who do not care.
> >
> > As far as I can see a contiguous output was not guaranteed for
> > "keeporder", but nobody probably ever used keeporder....
> 
> I don't understand what these have to do with each other. If vdot is
> going to ravel then yes, it certainly needs to control the order of
> the raveling. (And there are lots of functions like this that
> implicitly ravel in numpy, for better or worse... Do we need to audit
> any others?) But this is "virtual" order that matters, right, nothing
> to do with storage order?
> 

No, this change also potentially changed the contiguity of the output,
which does matter to vdot.
`.reshape(-1)` will be a view for arrays such as `np.zeros((4, 4,
4)[..., 0]`. However, before, `ravel` would always return a *contiguous*
chunk of memory (except for "keeporder"), so for the above case a copy
would be forced.

I think I underestimated the potential risk at the time, while I always
thought of `ravel` to create a view whenever possible (and the
documentation reads like it does), it never did and probably we should
be careful to avoid possible problems for C-interfacing code.

Asking everyone to use `reshape(-1)` instead if they want to have views
whenever possible, is maybe not as pretty sometimes, but safe.
I doubt "keeporder" should be special though, so I think I would also
add forcing of a copy there.

- Sebastian


> -n
> 
> _______________________________________________
> NumPy-Discussion mailing list
> NumPy-Discussion@scipy.org
> https://mail.scipy.org/mailman/listinfo/numpy-discussion

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
NumPy-Discussion mailing list
NumPy-Discussion@scipy.org
https://mail.scipy.org/mailman/listinfo/numpy-discussion

Reply via email to