INPLACE_ADD and INPLACE_MULTIPLY oddities in ceval.c
If you have Numeric or numpy installed try this: #import Numeric as N import numpy as N a = range(10) b = N.arange(10) a.__iadd__(b) print a Result: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9] Contrast the returned output with import numpy as N a = range(10) b = N.arange(10) a += b print a Result: [ 0 2 4 6 8 10 12 14 16 18] Having "a+=b" and "a.__iadd__(b)" do different things seems like an unfortunate bug. It seems to me that the problem is that the INPLACE_ADD and INPLACE_MULTIPLY cases in ceval.c use PyNumber_InPlace<YYY> instead of trying PySequence_InPlace<YYY> when the object doesn't support the in-place number protocol. I could submit a patch if there is agreement that this is a problem. -Travis
On 3/27/06, Travis Oliphant <oliphant.travis@ieee.org> wrote:
If you have Numeric or numpy installed try this:
#import Numeric as N import numpy as N
a = range(10) b = N.arange(10)
a.__iadd__(b)
print a
Result:
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
Contrast the returned output with
import numpy as N
a = range(10) b = N.arange(10)
a += b
print a
Result:
[ 0 2 4 6 8 10 12 14 16 18]
Having "a+=b" and "a.__iadd__(b)" do different things seems like an unfortunate bug.
It seems to me that the problem is that the INPLACE_ADD and INPLACE_MULTIPLY cases in ceval.c use PyNumber_InPlace<YYY> instead of trying PySequence_InPlace<YYY> when the object doesn't support the in-place number protocol.
I could submit a patch if there is agreement that this is a problem.
Well how is the interpreter to know whether += meanse numeric add or sequence add in any particular case? Shouldn't it be numpy's responsibility to implement both operations identically? (It's a design bug in Python that the + operation has two implementation slots. But it's difficult to fix that until Python 3000. It *will* be fixed there.) -- --Guido van Rossum (home page: http://www.python.org/~guido/)
Guido van Rossum wrote:
On 3/27/06, Travis Oliphant <oliphant.travis@ieee.org> wrote:
If you have Numeric or numpy installed try this:
#import Numeric as N import numpy as N
a = range(10) b = N.arange(10)
a.__iadd__(b)
print a
Result:
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
Contrast the returned output with
import numpy as N
a = range(10) b = N.arange(10)
a += b
print a
Result:
[ 0 2 4 6 8 10 12 14 16 18]
Having "a+=b" and "a.__iadd__(b)" do different things seems like an unfortunate bug.
It seems to me that the problem is that the INPLACE_ADD and INPLACE_MULTIPLY cases in ceval.c use PyNumber_InPlace<YYY> instead of trying PySequence_InPlace<YYY> when the object doesn't support the in-place number protocol.
I could submit a patch if there is agreement that this is a problem.
Well how is the interpreter to know whether += meanse numeric add or sequence add in any particular case?
Shouldn't it be numpy's responsibility to implement both operations identically?
"a" is a list object. Travis's question is, "why doesn't the list object implement both operations identically?" I think the answer is, "it never gets a chance to." When a numpy array is on the LHS of an inplace operation, we always want the numeric add. numpy arrays can't be extended inplace like a list, and a lone "+" is always a numeric operation for arrays, never a sequence operation. When numpy arrays are on the LHS of an inplace operation, everything works consistently. The issue seems to arise from the fact that PyNumber_InPlaceAdd tries to coerce the LHS and the RHS to a common numerical type first before trying PySequence_InPlaceConcat. Lists of numbers coerce to numpy arrays just fine, so this code path is taken, and nothing tests for the sq_inplace_concat or sq_concat slots. Personally, I'm willing to just document it in the numpy documentation as a rarely-seen gotcha. This behavior pretty much affects just us. -- Robert Kern robert.kern@gmail.com "I have come to believe that the whole world is an enigma, a harmless enigma that is made terrible by our own mad attempt to interpret it as though it had an underlying truth." -- Umberto Eco
Guido van Rossum wrote:
On 3/27/06, Travis Oliphant <oliphant.travis@ieee.org> wrote:
If you have Numeric or numpy installed try this:
#import Numeric as N import numpy as N
a = range(10) b = N.arange(10)
a.__iadd__(b)
print a
Result:
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
Contrast the returned output with
import numpy as N
a = range(10) b = N.arange(10)
a += b
print a
Result:
[ 0 2 4 6 8 10 12 14 16 18]
Having "a+=b" and "a.__iadd__(b)" do different things seems like an unfortunate bug.
It seems to me that the problem is that the INPLACE_ADD and INPLACE_MULTIPLY cases in ceval.c use PyNumber_InPlace<YYY> instead of trying PySequence_InPlace<YYY> when the object doesn't support the in-place number protocol.
I could submit a patch if there is agreement that this is a problem.
Well how is the interpreter to know whether += meanse numeric add or sequence add in any particular case?
I can see that '+' (and '*') having two different implementation slots is a design bug. However, it seems prudent that both += and .__iadd__ should have identical behavior regardless. Whatever mechanism is used to resolve the conflict for __iadd__, the same mechanism should be used for for +=. I don't think this is a NumPy-only issue. Any object that defines addition that works with lists will have similar problems. I think this can be fixed easily by first checking the sequence slot for a sq_concat function before calling PyNumber_InPlaceAdd. All I'm asking for is that a += b have the same behavior as a.__iadd__(b). That seems like desireable behavior to me. -Travis
On 3/27/06, Travis E. Oliphant <oliphant.travis@ieee.org> wrote:
Guido van Rossum wrote:
On 3/27/06, Travis Oliphant <oliphant.travis@ieee.org> wrote:
If you have Numeric or numpy installed try this:
#import Numeric as N import numpy as N
a = range(10) b = N.arange(10)
a.__iadd__(b)
print a
Result:
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
Contrast the returned output with
import numpy as N
a = range(10) b = N.arange(10)
a += b
print a
Result:
[ 0 2 4 6 8 10 12 14 16 18]
Having "a+=b" and "a.__iadd__(b)" do different things seems like an unfortunate bug.
It seems to me that the problem is that the INPLACE_ADD and INPLACE_MULTIPLY cases in ceval.c use PyNumber_InPlace<YYY> instead of trying PySequence_InPlace<YYY> when the object doesn't support the in-place number protocol.
I could submit a patch if there is agreement that this is a problem.
Well how is the interpreter to know whether += meanse numeric add or sequence add in any particular case?
I can see that '+' (and '*') having two different implementation slots is a design bug. However, it seems prudent that both += and .__iadd__ should have identical behavior regardless. Whatever mechanism is used to resolve the conflict for __iadd__, the same mechanism should be used for for +=.
I don't think this is a NumPy-only issue. Any object that defines addition that works with lists will have similar problems.
I think this can be fixed easily by first checking the sequence slot for a sq_concat function before calling PyNumber_InPlaceAdd.
All I'm asking for is that a += b have the same behavior as a.__iadd__(b). That seems like desireable behavior to me.
Correct. Robert Kern's analysis is correct: when you make the call explicitly, a.__iadd__(b) invokes the list.__iadd__ method; this is effectively an alias for list.extend. I take back whatever I said about numpy. So for consistency we want a += b to also execute a.__iadd__. The opcode calls PyNumber_InplaceAdd; I think that PyNumber_InplaceAdd (and PySequence_InplaceConcat, if it exists) should test for both the numeric and the sequence augmented slot of the left argument first; then they should try both the numeric and sequence non-augmented slot of the left argument; and then the numeric non-augmented slot of the right argument. Coercion should not be attempted at all. The question is, can we do this in 2.5 without breaking backwards compatibility? Someone else with more time should look into the details of that. -- --Guido van Rossum (home page: http://www.python.org/~guido/)
Hi, On Mon, Mar 27, 2006 at 08:00:09PM -0800, Guido van Rossum wrote:
So for consistency we want a += b to also execute a.__iadd__. The opcode calls PyNumber_InplaceAdd; I think that PyNumber_InplaceAdd (and PySequence_InplaceConcat, if it exists) should test for both the numeric and the sequence augmented slot of the left argument first; then they should try both the numeric and sequence non-augmented slot of the left argument; and then the numeric non-augmented slot of the right argument. Coercion should not be attempted at all.
The question is, can we do this in 2.5 without breaking backwards compatibility? Someone else with more time should look into the details of that.
I agree that there is a bug. There is more than one inconsistency left around here, though. Fixing one might expose the next one... For example, if -- as the documention says -- the expression 'a + b' would really try all slots corresponding to a.__add__(b) first and then fall back only if the slots return NotImplemented, then we'd also have to fix the following to return NotImplemented:
[].__add__(5) TypeError: can only concatenate list (not "int") to list
and then we have no place to put that nice error message. Nevertheless I think we should fix all this for consistency. I can try to give it a good look. I don't think many programs would break if the change goes into 2.5, but there are some C extension modules out there abusing the inner details of the type slots in unpredictable ways... A bientot, Armin
On 3/28/06, Armin Rigo <arigo@tunes.org> wrote:
Hi,
On Mon, Mar 27, 2006 at 08:00:09PM -0800, Guido van Rossum wrote:
So for consistency we want a += b to also execute a.__iadd__. The opcode calls PyNumber_InplaceAdd; I think that PyNumber_InplaceAdd (and PySequence_InplaceConcat, if it exists) should test for both the numeric and the sequence augmented slot of the left argument first; then they should try both the numeric and sequence non-augmented slot of the left argument; and then the numeric non-augmented slot of the right argument. Coercion should not be attempted at all.
The question is, can we do this in 2.5 without breaking backwards compatibility? Someone else with more time should look into the details of that.
I agree that there is a bug. There is more than one inconsistency left around here, though. Fixing one might expose the next one... For example, if -- as the documention says -- the expression 'a + b' would really try all slots corresponding to a.__add__(b) first and then fall back only if the slots return NotImplemented, then we'd also have to fix the following to return NotImplemented:
[].__add__(5) TypeError: can only concatenate list (not "int") to list
and then we have no place to put that nice error message.
Nevertheless I think we should fix all this for consistency. I can try to give it a good look. I don't think many programs would break if the change goes into 2.5, but there are some C extension modules out there abusing the inner details of the type slots in unpredictable ways...
Thanks for looking into this! C extensions are my main worry -- OTOH if += for a list can already passes arbitrary types as the argument, then any extension types should be ready to expect this, right? -- --Guido van Rossum (home page: http://www.python.org/~guido/)
Hi all, On Tue, Mar 28, 2006 at 09:50:49AM -0800, Guido van Rossum wrote:
C extensions are my main worry -- OTOH if += for a list can already passes arbitrary types as the argument, then any extension types should be ready to expect this, right?
Yes, I don't think C extensions are going to segfault. My worry is about returning a different result than before. Actually I believe the problem is not specific to C extensions. Here are some typical behavior changes that could be observed in pure Python already: class X(object): def __radd__(self, other): return 42 def __iter__(self): return iter("xyz") def __rmul__(self, other): return 42 def __index__(self): return 5 t = [] t += X() print t # current: 42 new: ['x', 'y', 'z'] print [1] * X() # current: 42 new: [1, 1, 1, 1, 1] Another visible difference is that the __add__/__iadd__/__mul__/__imul__ methods of lists, tuples, strings etc., will return NotImplemented instead of raising the TypeError themselves. This could impact user subclasses of these built-in types trying to override and call the super methods, not expecting a NotImplemented result (a reason why NotImplemented should have been an exception in the first place IMHO). (A different bug I found is that [1].__mul__(X()) with an __index__able class X currently raises TypeError, even though [1]*X() works just fine.) This seems to be it on the incompatibility side. I'd vote for the change anyway because the language specs -- as well as PyPy and probably all Python implementations other than CPython -- don't have this double-slot inconsistency and already show the "new" behavior. For what it's worth no CPython test breaks on top of PyPy because of this. If this change is accepted I'll submit a patch for 2.5. A bientot, Armin
On 3/29/06, Armin Rigo <arigo@tunes.org> wrote:
Hi all,
On Tue, Mar 28, 2006 at 09:50:49AM -0800, Guido van Rossum wrote:
C extensions are my main worry -- OTOH if += for a list can already passes arbitrary types as the argument, then any extension types should be ready to expect this, right?
Yes, I don't think C extensions are going to segfault. My worry is about returning a different result than before. Actually I believe the problem is not specific to C extensions. Here are some typical behavior changes that could be observed in pure Python already:
class X(object): def __radd__(self, other): return 42 def __iter__(self): return iter("xyz") def __rmul__(self, other): return 42 def __index__(self): return 5
t = [] t += X() print t # current: 42 new: ['x', 'y', 'z'] print [1] * X() # current: 42 new: [1, 1, 1, 1, 1]
Another visible difference is that the __add__/__iadd__/__mul__/__imul__ methods of lists, tuples, strings etc., will return NotImplemented instead of raising the TypeError themselves. This could impact user subclasses of these built-in types trying to override and call the super methods, not expecting a NotImplemented result (a reason why NotImplemented should have been an exception in the first place IMHO).
(A different bug I found is that [1].__mul__(X()) with an __index__able class X currently raises TypeError, even though [1]*X() works just fine.)
This seems to be it on the incompatibility side. I'd vote for the change anyway because the language specs -- as well as PyPy and probably all Python implementations other than CPython -- don't have this double-slot inconsistency and already show the "new" behavior. For what it's worth no CPython test breaks on top of PyPy because of this.
If this change is accepted I'll submit a patch for 2.5.
I trust you in these matters. Go for it. -- --Guido van Rossum (home page: http://www.python.org/~guido/)
Armin Rigo wrote:
[].__add__(5) TypeError: can only concatenate list (not "int") to list
Would that be much of a loss? It doesn't really give you much more information than something like Unsupported operand types for '+': list, int and what it does give is based on the assumption that concatenation is what the user has in mind. He might just as easily have been thinking of addition, or something else entirely. -- Greg
Travis E. Oliphant wrote:
I think this can be fixed easily by first checking the sequence slot for a sq_concat function before calling PyNumber_InPlaceAdd.
However, if this *is* fixed, it looks like it's going to break NumPy, in the sense that it will no longer be able to force an arithmetic interpretation for "+" when adding a sequence to an array. Greg
Greg Ewing wrote:
Travis E. Oliphant wrote:
I think this can be fixed easily by first checking the sequence slot for a sq_concat function before calling PyNumber_InPlaceAdd.
However, if this *is* fixed, it looks like it's going to break NumPy, in the sense that it will no longer be able to force an arithmetic interpretation for "+" when adding a sequence to an array.
Well, it won't break any "documented" behavior of NumPy which does not use the sq_concat (or sq_concat_inplace) slot at all. I can't imaging anybody relying on an in-place operations to return a "different" object, but we could make the change and run all the NumPy/SciPy tests to see what happens. But, you are right that w = [1,2,3,4] w += array([1,2,3,4]) would no longer make w equal to array([2,4,6,8]). But, this seems like a very good thing to me, as such behavior is actually quite hard to explain except as "its a bug" -Travis
Travis E. Oliphant wrote:
I can't imaging anybody relying on an in-place operations to return a "different" object, but we could make the change and run all the NumPy/SciPy tests to see what happens.
I'm really thinking more about the non-inplace operators. If nb_add and sq_concat are collapsed into a single slot, it seems to me that if you do a = [1, 2, 3] b = array([4, 5, 6]) c = a + b then a will be asked "Please add yourself to b", and a will say "Okay, I know how to do that!" and promptly concatenate itself with b. This would be very different from the current behaviour of Numeric arrays. I don't know whether Numeric users would consider it a serious problem or not, but I think we need to consider the implications before charging ahead too fast with slot unification. -- Greg
Hi Greg, On Wed, Mar 29, 2006 at 12:38:55PM +1200, Greg Ewing wrote:
I'm really thinking more about the non-inplace operators. If nb_add and sq_concat are collapsed into a single slot, it seems to me that if you do
a = [1, 2, 3] b = array([4, 5, 6]) c = a + b
then a will be asked "Please add yourself to b", and a will say "Okay, I know how to do that!" and promptly concatenate itself with b.
No: there is a difference between + and += for lists. You can only concatenate exactly a list to a list. Indeed:
[].__add__((2, 3)) TypeError: can only concatenate list (not "tuple") to list
By contrast, list += is like extend() and accepts any iterable. So if we provide a complete fix, [].__add__(x) will be modified to return NotImplemented instead of raising TypeError if x is not a list, and then [1,2,3]+array([4,5,6]) will fall back to array.__radd__() as before. I'll try harder to see if there is a reasonable example whose behavior would change... A bientot, Armin
Armin Rigo wrote:
Hi Greg,
On Wed, Mar 29, 2006 at 12:38:55PM +1200, Greg Ewing wrote:
I'm really thinking more about the non-inplace operators. If nb_add and sq_concat are collapsed into a single slot, it seems to me that if you do
a = [1, 2, 3] b = array([4, 5, 6]) c = a + b
then a will be asked "Please add yourself to b", and a will say "Okay, I know how to do that!" and promptly concatenate itself with b.
No: there is a difference between + and += for lists. You can only concatenate exactly a list to a list. Indeed:
[].__add__((2, 3)) TypeError: can only concatenate list (not "tuple") to list
By contrast, list += is like extend() and accepts any iterable. So if we provide a complete fix, [].__add__(x) will be modified to return NotImplemented instead of raising TypeError if x is not a list, and then [1,2,3]+array([4,5,6]) will fall back to array.__radd__() as before.
Ouch. Assuming the same path is followed with tuples, I think that this means the following behaviour will continue:
t = (1,2,3) a = array([4,5,6]) t += a t array([5, 7, 9])
That's not particularly desirable. There's not much to be done about it short of adding __iadd__s everywhere, which is probably brittle and unfriendly. And, admittedly this is a corner case that's very rarely going to cause trouble. Still, perhaps for Py3K it's worth considering if PyNumber_InplaceAdd should only call __iadd__ and __add__, not __radd__. Thus giving the target object complete control during inplace adds. Similarly for other inplace operations, of course. I'm not certain that all of the consequences of this change would be benign, but it's something to consider.
I'll try harder to see if there is a reasonable example whose behavior would change...
Regards, Tim Hochberg
On 3/29/06, Tim Hochberg <tim.hochberg@ieee.org> wrote:
Ouch. Assuming the same path is followed with tuples, I think that this means the following behaviour will continue:
t = (1,2,3) a = array([4,5,6]) t += a t array([5, 7, 9])
That's not particularly desirable.
Why not? It doesn't really differ much from the following example IMO:
x = 3 x += 0.5 x 3.5
-- --Guido van Rossum (home page: http://www.python.org/~guido/)
Tim Hochberg wrote:
Still, perhaps for Py3K it's worth considering if PyNumber_InplaceAdd should only call __iadd__ and __add__, not __radd__. Thus giving the target object complete control during inplace adds.
That's probably reasonable, although it would break the conceptual notion that a += b is equivalent to a = a + b when a can't be modified in-place. Greg
On 3/29/06, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Tim Hochberg wrote:
Still, perhaps for Py3K it's worth considering if PyNumber_InplaceAdd should only call __iadd__ and __add__, not __radd__. Thus giving the target object complete control during inplace adds.
That's probably reasonable, although it would break the conceptual notion that a += b is equivalent to a = a + b when a can't be modified in-place.
No, it's not reasonable, because this would fail: x = 5 x += 0.5 -- --Guido van Rossum (home page: http://www.python.org/~guido/)
Hi Tim, On Wed, Mar 29, 2006 at 08:45:10AM -0700, Tim Hochberg wrote:
Ouch. Assuming the same path is followed with tuples, I think that this means the following behaviour will continue:
t = (1,2,3) a = array([4,5,6]) t += a t array([5, 7, 9])
I fell into the same trap at first, but no: in fact, only lists have a special in-place addition among all the built-in objects. Tuples fall back to the normal addition, which means that you can only add tuples to tuples: >>> t = (1,2,3) >>> t += [4,5,6] TypeError: can only concatenate tuple (not "list") to tuple >>> t += array([4,5,6]) TypeError: ... This is current behavior and it wouldn't change. A bientot, Armin.
Hi Tim, Oups, sorry. I only just realized my mistake and the meaning of your message. On Thu, Mar 30, 2006 at 09:27:02AM +0200, Armin Rigo wrote:
>>> t = (1,2,3) >>> t += [4,5,6] TypeError: can only concatenate tuple (not "list") to tuple
>>> t += array([4,5,6]) TypeError: ...
This is current behavior and it wouldn't change.
I'm pasting untested bits of code. Indeed, as you point out: >>> t = (1,2,3) >>> t += array([4,5,6]) >>> t array([5, 7, 9]) and it would remain so after the fix. I still think the fix is a good thing, and the above is an issue at a different level. It's somehow the "fault" of list.__iadd__ and list.__imul__, which are oddballs -- before the introduction of set objects, it was the single place in the whole library of built-in types where in-place behavior was different from normal behavior. It would require an official language extension to say that for all sequences, += is supposed to accept any iterable (which may or may not be a good thing, I have no opinion here). Otherwise, I'd just ignore the whole sub-issue, given that 'tuple += array' returning an array is just correct language-wise and doesn't look like a trap for bad surprises -- if the user expected a tuple but gets an array, most tuple-like operations will work just fine on the array, except hashing, which gives a clean TypeError. A bientot, Armin
Armin Rigo wrote:
So if we provide a complete fix, [].__add__(x) will be modified to return NotImplemented instead of raising TypeError if x is not a list, and then [1,2,3]+array([4,5,6]) will fall back to array.__radd__() as before.
Ah, okay. That seems like it would work. -- Greg
participants (7)
-
Armin Rigo -
Greg Ewing -
Guido van Rossum -
Robert Kern -
Tim Hochberg -
Travis E. Oliphant -
Travis Oliphant