Bug 15722 – std.algorithm sum should favour speed

Status
NEW
Severity
enhancement
Priority
P4
Component
phobos
Product
D
Version
D2
Platform
x86_64
OS
Windows
Creation time
2016-02-25T10:33:59Z
Last change time
2024-12-01T16:26:05Z
Keywords
performance
Assigned to
No Owner
Creator
adamsibson
Moved to GitHub: phobos#9675 →

Comments

Comment #0 by adamsibson — 2016-02-25T10:33:59Z
From the Learn forum thread https://forum.dlang.org/post/[email protected] it became apparent that sum is four times slower (on DMD and LDC) than the naive loop sum method and similarly slower than reduce!((a, b) => a + b). Would it not be better to have a fast, basic implementation in algorithm and a slower, more accurate version in a sub-library? The performance gap is too large to justify the current version being the standard. I would suggest that most users just want a trivial sum function and the more technical users are capable of finding and using the more accurate version, especially if we properly document the library. The accuracy of the basic version is also not terrible, it's usually going to be good enough. This is not playing fast and loose with accuracy just for the sake of a few percent speed. This kind of poor performance iceberg reflects poorly on D for new users. It's not the right out of box experience.
Comment #1 by cpicard — 2016-02-25T12:24:09Z
I would rather have a comment in the doc saying "Pairwise summation is slower than naive one, for performance use reduce!((a, b) => a+b)", adding a new function for that seems overkill.
Comment #2 by adamsibson — 2016-02-25T13:40:21Z
(In reply to Cédric Picard from comment #1) > I would rather have a comment in the doc saying "Pairwise summation is > slower than naive one, for performance use reduce!((a, b) => a+b)", adding a > new function for that seems overkill. Why? There seems to be a strong resistance to convenience functions in the library, one that is unnecessary. There is a significant difference between arr.sum and arr.reduce!((a, b) => a + b) in usability. In writing this post I initially forgot I have do put a, b in an additional set of brackets, it's complicated and not particularly user-friendly.
Comment #3 by cpicard — 2016-02-26T21:09:48Z
(In reply to adamsibson from comment #2) > (In reply to Cédric Picard from comment #1) > > I would rather have a comment in the doc saying "Pairwise summation is > > slower than naive one, for performance use reduce!((a, b) => a+b)", adding a > > new function for that seems overkill. > > Why? There seems to be a strong resistance to convenience functions in the > library, one that is unnecessary. There is a significant difference between > arr.sum and arr.reduce!((a, b) => a + b) in usability. In writing this post > I initially forgot I have do put a, b in an additional set of brackets, it's > complicated and not particularly user-friendly. I don't feel like I'm blindly refusing something because it's a convenience function. The thing is, how do you want to do it? First of all, for comparison, what exists today: [1, 2, 3].reduce!"a+b"; // Not pairwise, considered bad code... meh. [1, 2, 3].reduce!((a, b) => a+b); // Not pairwise [1, 2, 3].reduce!sum; // Pairwise Add a "sum" overload that takes a range? It would be feasible but ambiguous: [1, 2, 3].sum // Not pairwise [1, 2, 3].reduce!sum // Pairwise, non obvious at all Add a new function? "sum" is already taken and changing it would only break code uselessly (and it would be hard to explain that "sum" now means something slightly different and that the old meaning is now sumPairwise) so you'd have to add something like "simpleSum"... Feasible but not so convenient for a convenience function. Add a flag in parameters that defaults to not-pairwise? Good for the default but really complicated for pairwise summation: sum(1, 2); [1, 2, 3].reduce!sum; sum(1, 2, summation.pairwise); [1, 2, 3].reduce!((a, b) => sum(a, b, summation.pairwise)); Add a compile-time flag that defaults to not-pairwise? Not satisfying IMHO. sum(1, 2); [1, 2, 3].reduce!sum; sum!(summation.pairwise)(1, 2); [1, 2, 3].reduce!(sum!(summation.pairwise)); Quite frankly, given the other choices, I think the default isn't that bad. Otherwise my preference would go to the overload but I'm sure it would be refused for being too ambiguous. So this is a real question: how would you like to see it done?
Comment #4 by john.loughran.colvin — 2016-03-09T15:08:05Z
Comment #5 by adamsibson — 2016-03-09T15:18:17Z
>John Colvin 2016-03-09 15:08:05 UTC >https://github.com/D-Programming-Language/phobos/pull/4069 Thanks John! Out of interest what is the impact on accuracy between the two methods?
Comment #6 by robert.schadek — 2024-12-01T16:26:05Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/phobos/issues/9675 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB