Le dimanche 04 janvier 2015 à 19:30 -0800, Greg Plowman a écrit : > I'm not sure how general this behaviour is with respect to other > types, but I have observed the following with a simple composite type: > > When summing a 1-element array of a simple composite type, the return > is a reference to the single element, rather than a copy of the single > element. > This seems at odds with the return value of summing an array of any > other size (even 0-element array if zero is defined) > Is this the desired behaviour? > > > type MyType > x::Int > end > > +(a::MyType, b::MyType) = MyType(a.x + b.x) > > A = [ MyType(i) for i = 1:5 ] > sumA = sum(A) > A[1].x = 77 > sumA # MyType(15), seems reasonable > > > B = [ MyType(i) for i = 1 ] > sumB = sum(B) > B[1].x = 88 > sumB # MyType(88), is this reasonable? > > sum(B) === B[1] # true > > > I guess for a copy to be returned (rather than a reference) would > need to define how to copy. Perhaps require on of the following? > * copy constructor > * copy() function > * zero() and return A[1] + zero() > > Would this be preferable to returning a reference? > > If I want to return a copy of single element, do I need to define my > own sum() function instead? No, looks like a bug to me. If you look at the definition of sum() that's used in your example, you'll see that it calls _mapreduce(), which in turn relies on the r_promote() function when only one element is passed. The code can be seen using edit(Base.r_promote).
For numbers, it indeed does r_promote(::AddFun, x::Number) = x + zero(x) But for other types, no copy is done: r_promote(::AddFun, x) = x This must be because zero() is not necessarily defined for arbitrary types. But for the sum to be defined, it looks to me that zero() must be defined too, and it looks like the code assumes that too. So I'm not sure why the definition for numbers is not used for everything. The only reason I can think of is that a copy may be costly for certain types, and it's not needed in most cases since the summation will create a new value in the general case. But as you noted this is not true when the array only contains one element. So it looks like the most efficient fix would be to copy only when n == 1 in _mapreduce(). This would also fix the same issue with more general mapreduce() operations, when the mapping function does not create a copy (like the identity function). Regards
