Commit 3496b1cf authored by Paal Kvamme's avatar Paal Kvamme
Browse files

Fix operator+= and operator-= in StatisticData. Old technical debt.

parent 25c2ca08
......@@ -33,7 +33,9 @@ namespace InternalZGY {
* but they will be set to 0 here just to have a consistent value.
*/
StatisticData::StatisticData()
: cnt_(0), inf_(0), sum_(0), ssq_(0), min_(0), max_(0)
: cnt_(0), inf_(0), sum_(0), ssq_(0)
, min_(std::numeric_limits<double>::infinity())
, max_(-std::numeric_limits<double>::infinity())
{
}
......@@ -69,7 +71,9 @@ StatisticData::StatisticData(count_type cnt, count_type inf, double sum, double
* range cannot grow.
*/
StatisticData::StatisticData(const count_type *bins, int nbins, double range_min, double range_max, bool is_int8)
: cnt_(0), inf_(0), sum_(0), ssq_(0), min_(0), max_(0)
: cnt_(0), inf_(0), sum_(0), ssq_(0)
, min_(std::numeric_limits<double>::infinity())
, max_(-std::numeric_limits<double>::infinity())
{
const double begin = range_min;
const double width = (range_max - range_min) / (nbins - 1);
......@@ -115,27 +119,28 @@ StatisticData::StatisticData(const count_type *bins, int nbins, double range_min
* determine whether the range is valid or not. Which is why (3) might
* fail, but is also a problem in its own right.
*
* TODO-@@@: Change to work more like a proper operator+= and operator-=.
* Unfortunately there seems to be code that depends on the old behavior.
* After the change, a StatisticData with all zeros is no longer a no-op
* when used either as lhs or rhs of operator+= and operator-=.
* This will now expand the range of lhs to include zero. I have changed
* the default constructor to set min>max to bring back the nop-op
* behavior. There is some risk that code creates an all-zero instance
* bu hand. Or worse, an instance with count zero and sun etc. garbage.
*/
StatisticData& StatisticData::operator+=(const StatisticData& other)
{
// If other.cnt_ == 0 then all counts should be 0 and the range is bogus.
// Except possibly the count of infinites.
if (other.cnt_ != 0) {
if (cnt_ != 0) {
// already have data, so the range needs to be combined.
if (other.min_ <= other.max_) {
if (min_ <= max_) {
min_ = std::min(min_, other.min_);
max_ = std::max(max_, other.max_);
} else {
// our own min/max is bogus since we don't have any samples yet.
}
else {
min_ = other.min_;
max_ = other.max_;
}
cnt_ += other.cnt_;
sum_ += other.sum_;
ssq_ += other.ssq_;
}
cnt_ += other.cnt_;
sum_ += other.sum_;
ssq_ += other.ssq_;
inf_ += other.inf_;
return *this;
}
......@@ -150,18 +155,19 @@ StatisticData& StatisticData::operator+=(const StatisticData& other)
*/
StatisticData& StatisticData::operator-=(const StatisticData& other)
{
if (other.cnt_ != 0) {
if (cnt_ != 0) {
if (other.min_ <= other.max_) {
if (min_ <= max_) {
min_ = std::min(min_, other.min_);
max_ = std::max(max_, other.max_);
} else {
}
else {
min_ = other.min_;
max_ = other.max_;
}
cnt_ -= other.cnt_;
sum_ -= other.sum_;
ssq_ -= other.ssq_;
}
cnt_ -= other.cnt_;
sum_ -= other.sum_;
ssq_ -= other.ssq_;
inf_ -= other.inf_;
return *this;
}
......@@ -230,8 +236,7 @@ void StatisticData::trimRange(const count_type *bins, int nbins, double range_mi
std::string
StatisticData::toString() const
{
if (getcnt() == 0 && getsum() == 0 && getssq() == 0 &&
(getmin() > getmax() || (getmin() == 0 && getmax() == 0)))
if (getcnt() == 0 && getsum() == 0 && getssq() == 0 && getinf() == 0 && getmin() > getmax())
return "empty";
std::stringstream ss;
ss << "min " << getmin() << " max " << getmax() << " cnt " << getcnt()
......
......@@ -13,6 +13,7 @@
// limitations under the License.
#include "test_all.h"
#include "test_utils.h"
#include "../impl/histogrambuilder.h"
#include <iostream>
......@@ -27,8 +28,8 @@
using namespace InternalZGY;
#define CPPUNIT_ASSERT(a) TEST_CHECK((a))
#define CPPUNIT_ASSERT_EQUAL(a,b) TEST_CHECK((a)==(b))
#define CPPUNIT_ASSERT_DOUBLES_EQUAL(a,b,eps) TEST_CHECK(std::abs((a)-(b)) <= (eps))
#define CPPUNIT_ASSERT_EQUAL(a,b) TEST_EQUAL(b,a)
#define CPPUNIT_ASSERT_DOUBLES_EQUAL(a,b,eps) TEST_EQUAL_FLOAT(b,a,eps)
namespace {
#if 0
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment