boostorg / histogram

Fast multi-dimensional generalized histogram with convenient interface for C++14
Boost Software License 1.0
316 stars 71 forks source link

implement rolling mean accumulator #269

Closed jbuonagurio closed 4 years ago

jbuonagurio commented 4 years ago

As requested in #250, this PR includes a basic rolling mean accumulator for trivial value types and a basic circular buffer implementation to eliminate the dependency on boost::circular_buffer.

Work-in-progress while awaiting support for arguments in storage_adaptor. Will of course help with unit tests as well!

jbuonagurio commented 4 years ago

I added the simple forward iterator to circular_buffer just for debugging purposes.

HDembinski commented 4 years ago

@jbuonagurio Do you agree with closing this?

jbuonagurio commented 4 years ago

@jbuonagurio Do you agree with closing this?

@HDembinski that's up to you, this PR was per your request in #250. They are important to me, but are rolling calculations important enough to other users to warrant an implementation at the library-level? I could continue work on this, adding support for variance calculation and compile-time fixed-size buffers. The circular buffer implementation is close to as simple as it gets unless I use something like std::deque which would probably impact performance, or depend on boost::circular_buffer (as Boost.Accumulators does).

HDembinski commented 4 years ago

I thought it is a good idea at first, but now it looks like we are rebuilding the rolling mean from Boost.Accumulators, which does not sound like a good investment of our time. The library-included accumulators mean, weighted_mean, etc are also duplications of code in Boost.Accumulatos, but they are so common and so simple that they can be implemented without relying on Boost.Accumulators explicitly. However, this should be rather the exception than the rule. Code duplication is bad.