rolling_mean returns incorrect result when using unsigned int

[Gareth White <gwhite@…>]

Using rolling_mean with a sample type of "unsigned int" leads to incorrect results. The following example outputs 1.43166e+009 instead of the expected 1.

#include <iostream>
#include <boost/accumulators/accumulators.hpp>
#include <boost/accumulators/statistics/stats.hpp>
#include <boost/accumulators/statistics/count.hpp>
#include <boost/accumulators/statistics/rolling_mean.hpp>

using namespace boost::accumulators;

int main(int argc, char** argv)
{
    accumulator_set<unsigned int, stats<tag::rolling_mean, tag::count>>  acc(tag::rolling_window::window_size = 3);

    acc(3);
    acc(2);
    acc(1);
    acc(0);

    std::cout << rolling_mean(acc) << std::endl;
}

The same problem happens if the sample type is "int" but you pass unsigned ints to the accumulator. For example, if you replace the above code with the following code, the result is the same:

    accumulator_set<int, stats<tag::rolling_mean, tag::count>>  acc(tag::rolling_window::window_size = 3);

    acc(3U);
    acc(2U);
    acc(1U);
    acc(0U);

I found this problem using Visual Studio 2010, after upgrading from Boost 1.44.0 to 1.58.0. With Boost 1.44.0, the above examples return 1.

[aholaway@…]

I'm also experiencing this issue after upgrading from Boost 1.49 to 1.58. I can work around the issue by using signed integers, but it would be nice to not have to.

[Martin]

Another workaround, which allows sticking with unsigned types, is to declare the accumulator_set using lazy_rolling_mean:

accumulator_set<unsigned int, stats<tag::lazy_rolling_mean, tag::count>>  acc(tag::rolling_window::window_size = 3);

Apparently, there are two implementation for rolling_mean and only one of them (immediate_rolling_mean, which - unfortunately - is the default) is affected by this issue.

As a quick fix, a static_assert(!std::is_unsigned<Sample>::value, ""); in immediate_rolling_mean_impl (statistics/rolling_mean.hpp) would prevent at least some troblesome use cases. However, even then it is possible to get incorrect results, if an unsigned type is fed to a signed accumulator_set. Due to C++ type promotion rules, a mixed signed/unsigned subtraction inside immediate_rolling_mean_impl would be performed as an unsigned operation and may underflow, causing weird behavior as described above.

To summarize, immediate_rolling_mean_impl needs some careful fixing. Until then it is probably better to make lazy_rolling_mean the default.

[Arjun]

what are the performance and memory implications of using lazy_rolling_mean vs immediate_rolling_mean?

[ojford@…]

I've fixed this and added a PR at ​https://github.com/boostorg/accumulators/pull/14