BUG: more on inplace reductions, fixes #615

[jaimefrio]

This is a follow up to #7407 and #7466. It fixes an error in the
comments (0 is the output and 1 is the input), moves them around so
they apply to both branches of the if/else, and changes memcpy to
memmove, since the memory segments can be overlapping, as pointed
out by Valgrind in a really old issue.

[seberg]

One of the few remaining valgrind warnings IIRC, always wondered if it mattered, but it can't hurt to fix in any case I guess. (My assumption would be that it can only matter in really weird cases).

Will merge this once my valgrind ran through with this applied (more curiosity, since it will not matter). Thanks for clearing up the comments!

[seberg]

Ah, there was another (or maybe this one was not visible anymore, not sure), warning about this in random.shuffle, which probably should be replaced as well? Can't figure out how bad it is. Then the last one which IIRC was some test in f2py using uninitialized arrays probably that I never figured out.

[seberg]

So, I don't mind putting this in, but more general. Should we just always prefer memmove even when the only reasonable "overlapping" case should be when both pointers are identical?

[jaimefrio]

I have not been able to find an example where memcpy vs memmove actually makes a difference. I was hoping that something like this:

>>> a = np.arange(6, dtype=np.double)
>>> b = a.view(np.complex128)
>>> c = a[1:3].view(np.complex128)
>>> np.add.reduceat(b, [0], out=c)
array([ 6.0+9.0j], dtype=complex128)

would do it, but as shown above it produces the correct result: it seems that we ask for 128 bit alignment for complex128, so that the destination array is update-if-copied, or I have no way of explaining why it is working. If I use longdouble and complex256 in the same example I at least get to see the overwriting of the first output affecting the result, i.e. the result is 5 + 9j, but not the expected effect of a naive memcpy, i.e. a 5 + 8j result.

So I guess this whole thing is more about "doing the right thing" than fixing any real issues: modern compilers/libraries seem to do a nice job of not letting you hurt yourself in the most stupid of ways.

[seberg]

Just been wondering myself about this. Basically, I think you have to do extreme things (like your view) to even get a chance of seeing it, and if you do that, you are probably amputating your foot anyway, whether with a gun, or a saw doesn't matter.
But, I can't imagine anyone can find a speed difference in any case unless they get out a microscope and forget there is a planet to consider. So why not silence a valgrind warning I guess...

Thanks Jaime.

[seberg]

Basically, I was just wondering if there is a clear answer to: "Should I bother to use memmove instead of memcpy in random.shuffle" even if there probably is no way to break it unless it is broken already.

[jaimefrio]

Unless you spend four months without submitting any PR while figuring out how to get approval from your employer to contribute to OS, and then go on a coding binge over Easter once you get approval, then there are probably a thousand things more worthy of your attention than memcpys in shuffle...