On 24 September 2015 at 09:17, Jens Jørgen Mortensen wrote:

...

...

...

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.

I can reproduce on Fedora with Numpy linked against Openblas. Numpy 1.9 in the same configuration works. The full test suite raises one error, but only due to lack of space when saving a large array.

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

Yeah, vdot uses `ravel`. That is correct, but it should only use it after making sure the input is C-order, or make sure the output of ravel is C-order (vdot was changed too in the mean time). So the fix will be adding the contiguity check in vdot. - Sebastian

commit 44877b36870ec2a0505c536a30b9fbf06a414c17 Author: Sebastian Berg Date: Wed Sep 3 18:33:06 2014 +0200

ENH: Allow ravel to reshape in place when possible

This fixes a lot of corner cases in reshape 'K' when the array is not contiguous in the first place, it was previously never ravelled in place.

Closes gh-5033

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

On Do, 2015-09-24 at 13:14 +0200, Sebastian Berg 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 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.

So to say, I am not quite sure yet. But if anyone is even a tiny bit nervous about doing less copying/not guaranteeing a contiguous result, lets change the definition of ravel to return a contiguous array always (and document it pointing people to reshape if they have no need for it).

As far as I can see a contiguous output was not guaranteed for "keeporder", but nobody probably ever used keeporder....

- Sebastian

...

commit 44877b36870ec2a0505c536a30b9fbf06a414c17 Author: Sebastian Berg Date: Wed Sep 3 18:33:06 2014 +0200

ENH: Allow ravel to reshape in place when possible

This fixes a lot of corner cases in reshape 'K' when the array is not contiguous in the first place, it was previously never ravelled in place.

Closes gh-5033

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

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

On Do, 2015-09-24 at 10:03 -0700, Nathaniel Smith wrote:

On Sep 24, 2015 4:14 AM, "Sebastian Berg" 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

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