Adjust MovingAverage UT to use MeanAccumulator and be @nogc #308
Conversation
source/mir/math/sum.d Outdated
| import mir.rc.array: RCI; | ||
| | ||
| MeanAccumulator!(T, summation) meanAccumulator; | ||
| Slice!(RCI!T) circularBuffer; |
There was a problem hiding this comment.
it should be double[], you may want to add @disable this(this)
There was a problem hiding this comment.
Happy to make this change, but I don't really understand the motivation for using double[] here, as opposed to a Slice, either just a simple contiguous one or the reference-counted one I used.
Honestly, I expected that making the change would have led to a gc allocation and was a little surprised when it didn't.
There was a problem hiding this comment.
A double[] doesn't really mean a GC-allocated memory. It is just a slice of memory. The idea is to allow a user to reuse existing memory. For example, it may be a stack-allocated memory and fallback to a manually allocated memory in case of a large size.
source/mir/math/sum.d Outdated
| | ||
| /// ma always keeps precise average of last 1000 elements | ||
| auto ma = new MovingAverage(linspace!double([1000], [0.0, 999]).array); | ||
| auto ma = MovingAverage!(double, Summation.precise)(linspace!double([1000], [0.0, 999]).rcslice); |
There was a problem hiding this comment.
You can allocate rcarray and pass its view [].
source/mir/math/sum.d Outdated
| import mir.ndslice.topology: linspace; | ||
| | ||
| class MovingAverage | ||
| struct MovingAverage(T, Summation summation) |
There was a problem hiding this comment.
Only precise summtation can be used for this kind of moving everage algorithm.
source/mir/math/sum.d Outdated
| swap(circularBuffer[frontIndex++], x); | ||
| summator -= x; | ||
| meanAccumulator.summator -= x; | ||
| frontIndex %= circularBuffer.length; |
There was a problem hiding this comment.
btw, in case you wish to add similar API to mir-stat:
// this line should be before `meanAccumulator.summator -= x;` frontIndex = frontIndex == circularBuffer.length ? 0 : frontIndex; meanAccumulator.summator -= x;This will be a much faster then %=
source/mir/math/sum.d Outdated
| Summator!(double, Summation.precise) summator; | ||
| import mir.math.stat: MeanAccumulator; | ||
| import mir.ndslice.slice: Slice; | ||
| import mir.rc.array: RCI; |
2 participants
I did this more too satisfy my own curiosity in terms of potentially adding the same functionality to
mir-statat some point in the future.One of the things I noticed is that
MeanAccumulatordoes not have+and-defined in the same way thatSummatordoes. It doesn't really have a big impact in this case because I avoid modifyingcount. However, it means the logic for adjusting it is inMovingAverageinstead ofMeanAccumulator. I would guess, without testing myself and not being 100% sure, that when compiling with-Othe adding and subtraction of count is elided.