-
-
Notifications
You must be signed in to change notification settings - Fork 694
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
[new concept under development] Python's fsum and others #2991
Conversation
Does using real for intermediates make it generate incorrect results? Or is it just a performance problem? |
The results is not exactly incorrect. The value should be rounded to the nearest representable floating-point number using the round-half-to-even rule. So precision loss can happen while truncated result from version(X86)
{
...
assert(nextDown(s) <= r && nextUp(s) >= r);
}
else
{
...
assert(s == r);
} |
Win_32 fails with msg:
|
Oh ok, makes sense.
You left a merge conflict marker in the makefile. |
Thanks, fixed. |
It works! |
@@ -1647,45 +1653,6 @@ unittest | |||
} | |||
|
|||
/** | |||
Computes accurate sum of binary logarithms of input range $(D r). | |||
*/ | |||
ElementType!Range sumOfLog2s(Range)(Range r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has this been released before? Is an alias needed to maintain compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be released in 2.067
. An alias is not needed since std.numeric.summation
is publicly imported in std.numeric
.
This is amazing and, honestly, I am not qualified to comment on any details. There is a lot in your code to teach me, both about D and numeric. That being said, I am all for including this. However, I'd try to restrict the public import to the numeric package as much as possible, to be able to change implementation details later on. As far as I can tell, the public functions are On the other hand, if you want to make a large part of the module available for others to use, it could be worthwhile to put this in std.experimental.numeric first. Again, I really like both the function |
The import std.range;
import std.algorithm.mutation : swap;
///Moving mean
class MovingAverage
{
Summator!double summator;
double[] circularBuffer;
size_t frontIndex;
///operation without rounding
void put(double x)
{
summator += x;
swap(circularBuffer[frontIndex++], x);
summator -= x;
frontIndex %= circularBuffer.length;
}
double avg() @property
{
return summator.sum() / circularBuffer.length;
}
this(double[] buffer)
{
assert(!buffer.empty);
circularBuffer = buffer;
summator = 0;
.put(summator, buffer);
}
}
/// ma always keeps pricese average of last 1000 elements
auto ma = new MovingAverage(iota(0.0, 1000.0).array);
assert(ma.avg == (1000*999/2) / 1000.0);
/// move by 10 elements
put(ma, iota(1000.0, 1010.0));
assert(ma.avg == (1010*1009/2 - 10*9/2) / 1000.0); |
@@ -812,6 +814,7 @@ T findRoot(T, DF, DT)(scope DF f, in T a, in T b, | |||
is(typeof(f(T.init)) == R, R) && isFloatingPoint!R | |||
) | |||
{ | |||
import std.math : fabs; // FIXME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please describe shortly what has to be fixed and why.
Nitpick: please, add
|
/++ | ||
Fast summation algorithm. | ||
+/ | ||
Fast, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum member names are generally lower-cased in Phobos.
Fixed |
alias Data = Tuple!(ptrdiff_t, "overflowDegree", F, "notFinities", F[], "partialSums");
Data toData(){...}
static typeof(this) createFromData(Data data) {...} or something else? |
What's blocking this? |
/++ | ||
Computes accurate geometric mean of input range $(D r). | ||
+/ | ||
//A worst-case error of roughly O(ε), independent of N. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this comment be part of the documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think no, it is the same behaviour like for normal ar.map!log2.sum
.
rebased |
ping @MartinNowak |
Can you provide some performance numbers, particularly for the default |
$(D Kahan) summation of user defined types. | ||
+/ | ||
appropriate, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should provide a brief and an extended overview for choosing the most suited algorithms, also explaining their drawbacks. Maybe make a table such as http://dlang.org/phobos/std_algorithm.html.
The are 1.5KLOC because unittests, special cases for complex numbers, Output Range abstraction, documentation. Andrei is preparing Phobos to work with |
Wouldn't it be best if summing could be used like this: auto walkToLast(R)(R r)
{
ElementType!R e;
foreach(el; r) e = el;
return e;
}
auto mean(R)(R r)
{
static if (hasLength!R)
{
return r.sum / r.length;
}
else
{
auto lengthAndSum = r.cumulativeSum.enumerate.walkToLast;
return lengthAndSum[1] / lengthAndSum[0];
}
} Background:
Moved example I had to a gist as I don't think it's actually relevant: https://gist.github.com/John-Colvin/8f6b63011e66c38067f5 Here is the important bitWhat I would really like to see:
/**
* Takes an input range and an output range with member "state" and
* returns a range of the value of state after each element of the input range
* is put in to the output range.
*/
struct Cumulative(IR, OR)
if (isOutputRange!OR && hasMember!(OR, "state") && isInputRange!IR)
{
IR ir;
OR or;
private bool empty_ = false;
auto front() @property
{
return or.state;
}
bool empty() @property
{
return empty_;
}
void popFront()
{
if(ir.empty) empty_ = true;
else
{
put(or, ir.front);
ir.popFront();
}
}
static if(isForwardRange!IR && hasMember!(OR, "save"))
auto save() @property
{
auto tmp = this;
tmp.or = or.save;
tmp.ir = save;
}
}
auto cumulative(IR, OR)(IR ir, OR or)
{
auto tmp = Cumulative!(IR, OR)(ir, or);
tmp.popFront();
return tmp;
}
template cumulativeSum(F, Summation summation = Summation.appropriate)
{
F cumulativeSum(Range)(Range r)
{
alias Sum = //the relevant output range for the given summation
return r.cumulative(Sum());
}
} |
/++ | ||
$(LUCKY Pairwise summation) algorithm. Range must be a finite sliceable range. | ||
+/ | ||
private F sumPairwise(Range, F = Unqual!(ForeachType!Range))(Range r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for sliceable or recursion:
/++
$(LUCKY Pairwise summation) algorithm. Range must be a finite range.
+/
private F sumPairwise(Range, F = Unqual!(ForeachType!Range))(Range data)
if (isInputRange!Range && !isInfinite!Range)
{
import core.bitop : bsf;
// Works for r with length < 2^^64, in keeping with the use of size_t
// elsewhere in std.algorithm and std.range on 64 bit platforms.
F[64] store = F.max;
size_t idx = 0;
foreach (i, el; enumerate(data))
{
store[idx] = el;
foreach (_; 0 .. (i+1).bsf)
{
store[idx - 1] += store[idx];
--idx;
}
++idx;
}
F s = store[idx - 1];
foreach_reverse (i; 0 .. idx - 1)
s += store[i];
return s;
}
(https://gist.github.com/John-Colvin/a57d1754f266fba96519)
P.S. there might be opportunities for optimised specialisations here, E.g. reading 32 elements at a time and doing an explicit pairwise sum, or maybe even SIMD.
P.P.S. don't expect the implementation above to give exactly the same result as the recursive implementation, they do slightly different summation trees.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will play with @John-Colvin ideas. Lets see how we can improve summation concept.
Note for pairwise sum as an output rangeReferring to the eager implementation at https://gist.github.com/John-Colvin/a57d1754f266fba96519 sum/state should return the reverse naïve sum of Note that, because |
Here's a faster pairwise sum: https://gist.github.com/John-Colvin/72a858b1687a2ab49209 Not sure how well it translates to a lazy setting. Having a 16 element cache that you eagerly fill and then exhaust is a possible approach. It's probably not worth doing it though, because in the more complex loops that are typical of lazy, range-based code, the unrolling is likely to be much less valuable. Nonetheless, it's a nice optimised version for when eager is OK. |
Could someone please describe this auto-tester error?
|
demangled: checkwhitespace.o(.text.pure nothrow @nogc @safe void std.numeric.summation.Summator!(uint, 0).Summator.put!(uint[]).put(uint[])+0x2f): In function `pure nothrow @nogc @safe void std.numeric.summation.Summator!(uint, 0).Summator.put!(uint[]).put(uint[]):'
: undefined reference to `std.numeric.summation.__array' |
Thanks @John-Colvin ! @MartinNowak, Does this error message means that |
Maybe this is a template emission bug? |
I don't think so. |
remove old commit rebase sum remove old commit remove trailing spaces fix rebase update imports remove spaces revert one import change remove geometricMean move `sum` function from std.algorithm to std.numeric.summation New pairwise algorithm for Summator + naive and fast algorithms for Summator simplified sumPrecise remove useless unittest fix import in std/algorithm/iteration update Quaternion minor generalization template simplification fix win*.mak change default algorithms for Summator fix sumPrecise add SIMD support remove unused import rename unused vars rework sum template and unittests update SummationAlgo _save_
Summation algorithms will be part of |
See the discussion #2513.
Example 0:
Example 1:
Example 2: