Add new function std.algorithm.iteration : cumulativeSum

[e-y-e]

To complement the addition of cumulativeFold (#3972), this function is a specialization for accurate summation. It uses Kahan summation algorithm where the given range has floating point elements, or the given seed is a floating point value.

Please review in detail as this is my first PR for a new function. I checked out the documentation and unittests of both cumulativeFold and sum for ideas on how to proceed with this function's docs & unittests but it is quite possible I have missed things.

[codecov-io]

Current coverage is 89.39% (diff: 94.54%)

Merging #4881 into master will increase coverage by <.01%

@@             master      #4881   diff @@
==========================================
  Files           124        124          
  Lines         76608      76663    +55   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          68476      68530    +54   
- Misses         8132       8133     +1   
  Partials          0          0          

Powered by Codecov. Last update 8e10b4b...08ed80b

[e-y-e]

Pinging @andralex : is it possible to get this PR reviewed or discussed?

[andralex]

@e-y-e was there traction in the forums?

[e-y-e]

No, I only mentioned it as an aside in the forums (https://forum.dlang.org/post/xvlvallwfxovoxvesccl@forum.dlang.org).

[e-y-e]

Obviously 2.072 has been and gone now, but I still think this is a valuable function for phobos as it fills in a logical gap:

X	Not specialized for accurate summation	Specialized for accurate summation
Provides no intermediate results	fold	sum
Provides intermediate results	cumulativeFold	cumulativeSum

I was surprised not to find it, I would certainly have a use case for it and would be glad if it was in phobos.

[andralex]

@e-y-e would be great to start a dedicated thread in the forum discussing motivation and applications. Thanks!

[e-y-e]

Ok can do, thanks for your time

[9il]

I think std.algorithm should not get new numeric code. First we need DMD to support pragma's for floating point control, then we need to merge mir.sum to Phobos and think what additional API like comulativeSum can be added.

[e-y-e]

If I am reading that bug report correctly, does this mean that the Kahan summation algorithm is not doing what it is supposed to under DMD? Looking at mir.sum I think you are right that it should be merged in some form or another (like many mir projects :) ).

[9il]

If I am reading that bug report correctly, does this mean that the Kahan summation algorithm is not doing what it is supposed to under DMD?

Yes

[e-y-e]

@andralex here is the forum post I made last week (be aware there are typos/mistakes in the post so read the follow up replies if confused): https://forum.dlang.org/thread/qpydmhlmxvbkolbouowf@forum.dlang.org

Not much activity really but no objections it seems.

I also tweaked some of the unittests so that I am satisfied with them. I could add more documented unittest examples such as the ones mentioned in the linked post if it is necessary.

[andralex]

thx!

[andralex]

s/an/An/

[schveiguy]

Little nits like this, you can now change yourself.

[schveiguy]

Auto-merge toggled on

[e-y-e]

Thanks everyone! :)

[9il]

? How we merge the PR with decision block label?

[schveiguy]

@9il Didn't know that was a thing? Andrei approved it, looks correct to me, I thought that should undo the block.

I see your complaints, but I don't see any reason why the Kahan summation algorithm is incorrect, looks identical to the wikipedia article. Your issue seems to stem from DMD generating incorrect code, which should be fixed of course, and then the correct thing will happen, right?

In other words, because DMD has issues with adding floats, this doesn't mean we should banish floating point math. We should just fix the issues.

[andralex]

@9il I hadn't seen your comment, sorry. At any rate, since we have sum here, it stands to reason that cumulativeSum belongs here as well.

We have one release cycle to get this right, so no worries that it's merged already. @9il could you please make sure you take this home? Does your mir.sum package offer a competing feature? What setup do you recommend?

[9il]

We have one release cycle to get this right, so no worries that it's merged already. @9il could you please make sure you take this home? Does your mir.sum package offer a competing feature? What setup do you recommend?

Issues with this PR:

1. Kahan summation does not tested properly (thanks to approxEqual).
2. Kahan summation does not work with current DMD (fix Issue 13474 - Discard excess precision for float and double (x87) dmd#6247 was not merged yet !)
3. It does not cover complex numbers (both std and native).
4. It does not cover user defined types with FloatingPoint-like behavior (for Quaternion for example)
5. KBN should be used for floating point and complex numbers instead of Kahan. It is faster and not less precise then Kahan.
6. Precise summation or an ability to add it into current API in the future it is not presented.
7. the string lambda should be used instead of (a, b) => a + b to reduce template bloat.

comulativeSum should be reverted and sum should be deprecated. After dlang/dmd#6247 be merger, we can add std.experimental.numeric.sum from mir.sum, which has OutputRange API for summation. An output range is more flexible for numeric algorithms, it covers comulativeSum functionality. If we really need comulativeSum sum it can be added too (20 LOC).

[andralex]

@9il alrighty, thanks for the 'splanations. I've merged dlang/dmd#6247 so that changes the landscape but just by a little bit.

I'll give you authority on this, feel free to undo this (no hard feelings @e-y-e and thanks for the work). Please do not deprecate existing sum, but you may of course improve its implementation and/or reduce it to an alias in another package.

If we have some functionality for cumulative sum there's no need for a convenience wrapper, it's not frequently needed.

Sounds good?

[9il]

Yes, thanks

[e-y-e]

No problem, I have no attachment to this code (especially if it's wrong) ;). It would have been nice if this was discussed before the PR was merged but here we are, and those are good reasons.