boostorg / histogram

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

[bug] weighted_mean variance calculation in += #310

Closed henryiii closed 3 years ago

henryiii commented 3 years ago

The same issue fixed in #308 with mean seems to be present in weighted_mean, as well:

#include <boost/histogram.hpp>
#include <boost/histogram/accumulators/ostream.hpp>
#include <iostream>

int main() {
  namespace bh = boost::histogram;

  bh::accumulators::weighted_mean a, b, c;
  a(1); a(2); a(3);

  b(5); b(6);

  c(1); c(2); c(3); c(5); c(6);

  auto d = a;
  d += b;

  std::cout << d << " " << c << " " << (a == b ? "Same" : "Different") << std::endl;
}
weighted_mean(5, 3.4, 0.625) weighted_mean(5, 3.4, 4.3) Different
henryiii commented 3 years ago

Unrelated: Why is weighted_mean& operator* not noexcept? Simple omission or intentional design?

henryiii commented 3 years ago

Exaclty the same fix fixes WeighedMean. I've fixed it in https://github.com/scikit-hep/boost-histogram/pull/537 - I can apply the same fix here if you'd like, @HDembinski, or you can do it, whatever you think is faster/easier to get in.

HDembinski commented 3 years ago

I suspected as much, but planned to implement this later.

HDembinski commented 3 years ago

Taken care of in #311 also the missing noexcept that you found.