signed overflow problem in weighted_sum on VC9.0

[Pavol Droba]

Hi,

I have spotted rather unexpected behaviour while using weighted_sum.

Here is the simple code:

// use namespaces
using namespace boost::accumulators;
using namespace std;

// create a int accumulator with std::size_t weight
accumulator_set<int, features<tag::weighted_sum>, std::size_t> Accum;

// Accumulate negative value with positive weight
Accum(-56, boost::accumulators::weight=3);

// Print the output
wcout << "sum: " << boost::accumulators::weighted_sum(Accum) << endl;

I was expecting to get result -168, however I got 4294967128.

I have traced the problem to weighted_sum_impl.

I don't know why, but

weighted_sum_impl<int, unsigned int, tag::sample>::weighted_sample

is resolved to unsigned int. I think this is plain wrong.

[Eric Niebler]

This is not a bug. The sample is multiplied by the weight. In C++, and int multiplied by an unsigned int is an unsigned int. (Try it and see.)

If you want the result to be an int, use an int for the weight type.

[Pavol Droba]

I understand that the current solution is technically correct. However I think that conceptually it is not.

An example: To calculate a weighted mean of signed integers, where weight represents sample cardinality, I would consider it quite natural to expect that the mean will be signed, since I'm accumulating signed integers. Even though the cardinality is natural (i.e. unsigned) number.

[Eric Niebler]

I understand your point, but I still disagree. I'm not going to get into guessing what a "better" type to hold the result is. It can be argued from the other side ... that using an int to hold the result of an (int*unsigned) can overflow and wrap. There is no good solution. Following the rules of C++ is simply the best (as in least surprising, easiest to explain) thing to do.