Comment #0 by bearophile_hugs — 2014-02-15T01:44:01Z
import std.algorithm: sum;
void main() {
int[] arr1;
auto arr2 = new int[arr1.sum];
}
With dmd 2.065beta3 gives:
test3.d(4,25): Error: cannot implicitly convert expression (sum(arr1)) of type long to uint
It seems a sum(int[]) gives a long. 99% of the cases this is not what I need.
Comment #1 by peter.alexander.au — 2014-02-15T05:49:52Z
This is by design in the code, presumably to avoid overflow (which you are usually so concerned about, what changed?)
I actually agree with you though. I'd expect sum!(int[]) to return int. In general, I'd expect sum!(T[]) to return typeof(T.init + T.init), and respect the language integral promotions.
A nice compromise might be to adjust the API:
int[] xs;
int sum1 = sum(xs);
long sum2 = sum!long(xs);
That way, you can specify what type to return. This also solves the problem of potential overflows with longs (just specify ReturnType = BigInt).
I'll assign this to Andrei for his thoughts.
Comment #2 by andrei — 2014-02-15T06:15:26Z
If sum returned int for ranges of int, it would be 100% identical to std.algorithm.reduce. It's worth returning a 64-bit quantity because (a) 64-bit summing is about as fast as 32-bit summing, (b) the result can be cast to int if needed.
I propose we close this.
Comment #3 by bearophile_hugs — 2014-02-15T06:32:29Z
(In reply to comment #1)
> This is by design in the code, presumably to avoid overflow (which you are
> usually so concerned about, what changed?)
I have tens of usages of sum in my code and no case I need a long result. So the Phobos sum halves my usability. And I need to keep my own sum() around.
> I actually agree with you though. I'd expect sum!(int[]) to return int. In
> general, I'd expect sum!(T[]) to return typeof(T.init + T.init), and respect
> the language integral promotions.
Right.
> A nice compromise might be to adjust the API:
>
> int[] xs;
> int sum1 = sum(xs);
> long sum2 = sum!long(xs);
>
> That way, you can specify what type to return. This also solves the problem of
> potential overflows with longs (just specify ReturnType = BigInt).
This is OK, but an alternative (and in my opinion better) solution is in Issue 12173:
long sum2 = xs.sum(0L);
Comment #4 by bearophile_hugs — 2014-02-15T06:38:04Z
(In reply to comment #2)
> If sum returned int for ranges of int, it would be 100% identical to
> std.algorithm.reduce.
If it's equal to what reduce does, it gives less surprises to the programmer. I am sure in Haskell you don't see a sum to act differently from reduce. One is defined on the other, for uniformity, simplicity and minimize surprises.
Also what do you want sum(long[]) to return on default? A "cent" perhaps?
> It's worth returning a 64-bit quantity because (a) 64-bit
> summing is about as fast as 32-bit summing, (b) the result can be cast to int
> if needed.
I am not so fond of casts. What if later I change the code and I am now summing floats instead? The cast(int) will remove the floating point part :-)
> I propose we close this.
Let's hear what others think first :-)
Comment #5 by timon.gehr — 2014-02-16T07:00:53Z
(In reply to comment #1)
> This is by design in the code, presumably to avoid overflow (which you are
> usually so concerned about, what changed?)
>
> I actually agree with you though. I'd expect sum!(int[]) to return int. In
> general, I'd expect sum!(T[]) to return typeof(T.init + T.init), and respect
> the language integral promotions.
> ...
+1.
(In reply to comment #2)
> If sum returned int for ranges of int, it would be 100% identical to
> std.algorithm.reduce.
Not yet, there are more special cases. (but a more general divide and conquer reduction algorithm might be an useful addition.)
BTW: D semantics do not seem to support Kahan summation. (Walter has expressed in the past that any operation might arbitrarily be performed at higher precision.)
> It's worth returning a 64-bit quantity because
> (a) 64-bit summing is about as fast as 32-bit summing,
> (b) the result can be cast to int if needed.
>
IMO it's not worth the special casing. (Another thing I find questionable: extending range capabilities can reduce precision.) Allowing specification of the summation type is enough.
Comment #6 by bearophile_hugs — 2014-02-18T03:23:57Z
(In reply to comment #1)
> presumably to avoid overflow (which you are
> usually so concerned about, what changed?)
This after very few seconds of run time prints the wrong result 3028092401290448384 instead of 4_294_967_295 * 5_000_000_000 = 21474836475000000000:
struct Uints {
ulong count;
bool empty() { return count == 0; }
void popFront() { count--; }
uint front() { return uint.max; }
}
void main() {
import std.stdio, std.algorithm;
Uints(5_000_000_000).sum.writeln;
}
I think sum() should be changed to return an int before dmd 2.065final is released.
Comment #7 by monarchdodra — 2014-02-19T13:51:58Z
(In reply to comment #0)
> import std.algorithm: sum;
> void main() {
> int[] arr1;
> auto arr2 = new int[arr1.sum];
> }
>
>
> With dmd 2.065beta3 gives:
>
> test3.d(4,25): Error: cannot implicitly convert expression (sum(arr1)) of type
> long to uint
>
> It seems a sum(int[]) gives a long. 99% of the cases this is not what I need.
Looks like I missed this when pulling. I even suggested this change:
https://github.com/D-Programming-Language/phobos/pull/1205/files#r9175954
I didn't realize the wordy code was actually doing something different. I would have disagreed with it had I seen it.
(In reply to comment #2)
> If sum returned int for ranges of int, it would be 100% identical to
> std.algorithm.reduce. It's worth returning a 64-bit quantity because (a) 64-bit
> summing is about as fast as 32-bit summing, (b) the result can be cast to int
> if needed.
>
> I propose we close this.
Wasn't the *point* for sum to be a specialization of reduce though? To be a more convenient and more accurate/efficient result? I don't think doing minor deviations as such is worth it.
(In reply to comment #3)
> This is OK, but an alternative (and in my opinion better) solution is in Issue
> 12173:
>
> long sum2 = xs.sum(0L);
I think this is a nice and simple solution. It perfectly fits with the idea that sum is reduce but specialized for numeric "+".
(In reply to comment #5)
> (In reply to comment #1)
> > I actually agree with you though. I'd expect sum!(int[]) to return int. In
> > general, I'd expect sum!(T[]) to return typeof(T.init + T.init), and respect
> > the language integral promotions.
> > ...
>
> +1.
>
> (In reply to comment #2)
> > It's worth returning a 64-bit quantity because
> > (a) 64-bit summing is about as fast as 32-bit summing,
> > (b) the result can be cast to int if needed.
> >
>
> IMO it's not worth the special casing. (Another thing I find questionable:
> extending range capabilities can reduce precision.) Allowing specification of
> the summation type is enough.
+1
+1
Comment #8 by andrei — 2014-02-25T09:55:00Z
Fine. I think if sum returned int, having it return long would have been submitted as an enhancement request :o).
I'm busy for the time being, could someone please make a pull request. Thanks.
Comment #9 by github-bugzilla — 2014-02-25T13:37:12Z