-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Should sum() of 1-element array return a new object? #9669
Comments
Have you tried to make the changes and run the tests? |
The solution suggested above is elegant, but it has a cost if doing Thus, another solution would be to copy |
I agree that there is seemingly unnecessary allocation for n==1. Having said that, I'm definitely for a more efficient solution. |
See the discussion towards the end of #8759. In many cases, not just It's very hard to convince me that |
Jeff, I agree that one can't please everyone, or even someone all of the time. Suppose Admittedly, this might be inefficient since a temporary object I don't think of the proposed change as an explicit copy that sum() is going out of its way make, but rather converting the return type of Perhaps the type is not closed under addition. In this case returning Doesn't consistency of return type count for anything? I do agree with your arguments in #8759 about indiscriminate copying and the dangers of library code trying to determine if and when to copy. Actually for my purpose, I just subtype from Cheers, Greg |
Wanting But if the only real reason to do |
For me, the real goal is not copying per se. The real goal is consistency of return types for sum(), and allowing the "calling" type to control this. I concede this objective might unrealistic, or at odds with other goals and design philosophies. I also get that if I really want a copy I should do Please note that I'm merely explaining a newcomers perspective to you, not trying to convince a developer of what's "right". Again, I'm not really invested in this change. However, it does seem that I have been invested in the explanation of my original query on julia-users, (from which I received generous help that was much appreciated, and culminated in advising me to raise an issue here) You have explained your position well enough, please feel free to close the issue. |
@GregPlowman, we definitely want consistency of return types; type stability is essential for performance, and we've put some effort into this for However, in your example above, the return type is consistent: it is always |
Agreed. But I think we'd be no worse off if we extended this to include non-obvious definitions of
Agreed. But maybe it could be seen as a non-type-related issue instead. Firstly, some context: My view of Intuitively one might expect
So is it unreasonable to expect I could of course:
And this is after first diagnosing the potential pitfall, then tracking it down to the inconsistency in I guess you might think that ref/copy is not of "functional or mathematical" concern because it's not about types. You might say its not relevant what is returned. In practice, however these are real programming concerns. I like the help for
Presumably these help tips are provided because it is relevant to users, and to provide clarity and prevent any gotchas. On the type stability part, I agree that under "usual" definitions of |
Closing since we can't change this now. |
Consider the following code:
sum() returns a reference to A[1] rather than a new object.
Also note that for 1-element arrays, sum() works even without +(::MyType, ::MyType) being defined.
See discussion here: https://groups.google.com/forum/?fromgroups=#!topic/julia-users/eh1OVunzYsw
Here's lines 44-47 of reduce.jl:
Possible fix is to take r_promote() currently defined for Numbers only, and use for all types:
Any type using sum() would need to define +(T, T), zero(T) and zero(Type{T}), which seems reasonable.
The text was updated successfully, but these errors were encountered: