improve error message for splitting failure (#152)

improve the logging of the error message reported in #142. 

relates #142

Author: Hendrik Muhs

include/maths/CAdaptiveBucketing.h

@@ -130,6 +130,9 @@ class MATHS_EXPORT CAdaptiveBucketing {
     TDoubleVec variances() const;
     //@}
 
+    //! Name of component
+    virtual std::string name() const = 0;
+
 protected:
     using TRestoreFunc = std::function<bool(core::CStateRestoreTraverser&)>;
     using TPersistFunc = std::function<void(core::CStatePersistInserter&)>;

include/maths/CCalendarComponentAdaptiveBucketing.h

@@ -32,7 +32,7 @@ class CSeasonalTime;
 //!
 //! DESCRIPTION:\n
 //! See CAdaptiveBucketing for details.
-class MATHS_EXPORT CCalendarComponentAdaptiveBucketing : public CAdaptiveBucketing {
+class MATHS_EXPORT CCalendarComponentAdaptiveBucketing final : public CAdaptiveBucketing {
 public:
     using TFloatMeanVarAccumulator = CBasicStatistics::SSampleMeanVar<CFloatStorage>::TAccumulator;
     using CAdaptiveBucketing::count;
@@ -97,6 +97,9 @@ class MATHS_EXPORT CCalendarComponentAdaptiveBucketing : public CAdaptiveBucketi
     //! Get the memory used by this component
     std::size_t memoryUsage() const;
 
+    //! Name of component
+    std::string name() const override;
+
 private:
     using TFloatMeanVarVec = std::vector<TFloatMeanVarAccumulator>;
 
@@ -110,28 +113,28 @@ class MATHS_EXPORT CCalendarComponentAdaptiveBucketing : public CAdaptiveBucketi
     //! bucket configuration.
     //!
     //! \param[in] endpoints The old end points.
-    void refresh(const TFloatVec& endpoints);
+    void refresh(const TFloatVec& endpoints) override;
 
     //! Check if \p time is in the this component's window.
-    virtual bool inWindow(core_t::TTime time) const;
+    bool inWindow(core_t::TTime time) const override;
 
     //! Add the function value to \p bucket.
-    virtual void add(std::size_t bucket, core_t::TTime time, double value, double weight);
+    void add(std::size_t bucket, core_t::TTime time, double value, double weight) override;
 
     //! Get the offset w.r.t. the start of the bucketing of \p time.
-    virtual double offset(core_t::TTime time) const;
+    double offset(core_t::TTime time) const override;
 
     //! Get the count in \p bucket.
-    virtual double bucketCount(std::size_t bucket) const;
+    double bucketCount(std::size_t bucket) const override;
 
     //! Get the predicted value for \p bucket at \p time.
-    virtual double predict(std::size_t bucket, core_t::TTime time, double offset) const;
+    double predict(std::size_t bucket, core_t::TTime time, double offset) const override;
 
     //! Get the variance of \p bucket.
-    virtual double variance(std::size_t bucket) const;
+    double variance(std::size_t bucket) const override;
 
     //! Split \p bucket.
-    virtual void split(std::size_t bucket);
+    void split(std::size_t bucket) override;
 
 private:
     //! The time provider.

include/maths/CSeasonalComponentAdaptiveBucketing.h

@@ -32,7 +32,7 @@ namespace maths {
 //!
 //! DESCRIPTION:\n
 //! See CAdaptiveBucketing for details.
-class MATHS_EXPORT CSeasonalComponentAdaptiveBucketing : public CAdaptiveBucketing {
+class MATHS_EXPORT CSeasonalComponentAdaptiveBucketing final : public CAdaptiveBucketing {
 public:
     using CAdaptiveBucketing::TFloatMeanAccumulatorVec;
     using TDoubleRegression = CRegression::CLeastSquaresOnline<1, double>;
@@ -130,6 +130,9 @@ class MATHS_EXPORT CSeasonalComponentAdaptiveBucketing : public CAdaptiveBucketi
     //! Get the memory used by this component
     std::size_t memoryUsage() const;
 
+    //! Name of component
+    std::string name() const override;
+
 private:
     using TSeasonalTimePtr = std::unique_ptr<CSeasonalTime>;
 
@@ -163,28 +166,28 @@ class MATHS_EXPORT CSeasonalComponentAdaptiveBucketing : public CAdaptiveBucketi
     //! bucket configuration.
     //!
     //! \param[in] endpoints The old end points.
-    void refresh(const TFloatVec& endpoints);
+    void refresh(const TFloatVec& endpoints) override;
 
     //! Check if \p time is in the this component's window.
-    virtual bool inWindow(core_t::TTime time) const;
+    bool inWindow(core_t::TTime time) const override;
 
     //! Add the function value at \p time.
-    virtual void add(std::size_t bucket, core_t::TTime time, double value, double weight);
+    void add(std::size_t bucket, core_t::TTime time, double value, double weight) override;
 
     //! Get the offset w.r.t. the start of the bucketing of \p time.
-    virtual double offset(core_t::TTime time) const;
+    double offset(core_t::TTime time) const override;
 
     //! The count in \p bucket.
-    virtual double bucketCount(std::size_t bucket) const;
+    double bucketCount(std::size_t bucket) const override;
 
     //! Get the predicted value for \p bucket at \p time.
-    virtual double predict(std::size_t bucket, core_t::TTime time, double offset) const;
+    double predict(std::size_t bucket, core_t::TTime time, double offset) const override;
 
     //! Get the variance of \p bucket.
-    virtual double variance(std::size_t bucket) const;
+    double variance(std::size_t bucket) const override;
 
     //! Split \p bucket.
-    virtual void split(std::size_t bucket);
+    void split(std::size_t bucket) override;
 
     //! Get the interval which has been observed at \p time.
     double observedInterval(core_t::TTime time) const;

lib/maths/CAdaptiveBucketing.cc

@@ -673,7 +673,9 @@ void CAdaptiveBucketing::maybeSplitBucket() {
                     CTools::safeCdfComplement(binomial, m_LargeErrorCounts[i - 1])};
                 m_LargeErrorCountSignificances.add({oneMinusCdf, i - 1});
             } catch (const std::exception& e) {
-                LOG_ERROR(<< "Failed to calculate splitting significance: " << e.what());
+                LOG_ERROR(<< "Failed to calculate splitting significance: " << e.what()
+                          << " interval = " << interval << " period = " << period
+                          << " type = " << this->name());
             }
         }
         if (m_LargeErrorCountSignificances.count() > 0) {

lib/maths/CCalendarComponentAdaptiveBucketing.cc

@@ -351,5 +351,10 @@ void CCalendarComponentAdaptiveBucketing::split(std::size_t bucket) {
     CBasicStatistics::scale(0.25, m_Values[bucket]);
     m_Values.insert(m_Values.begin() + bucket, m_Values[bucket]);
 }
+
+std::string CCalendarComponentAdaptiveBucketing::name() const {
+    return "Calendar[" + std::to_string(this->decayRate()) + "," +
+           std::to_string(this->minimumBucketLength()) + "]";
+}
 }
 }

lib/maths/CSeasonalComponentAdaptiveBucketing.cc

@@ -573,6 +573,11 @@ void CSeasonalComponentAdaptiveBucketing::split(std::size_t bucket) {
     m_Buckets.insert(m_Buckets.begin() + bucket, m_Buckets[bucket]);
 }
 
+std::string CSeasonalComponentAdaptiveBucketing::name() const {
+    return "Seasonal[" + std::to_string(this->decayRate()) + "," +
+           std::to_string(this->minimumBucketLength()) + "]";
+}
+
 double CSeasonalComponentAdaptiveBucketing::observedInterval(core_t::TTime time) const {
     return m_Time->regressionInterval(
         std::min_element(m_Buckets.begin(), m_Buckets.end(),

lib/maths/unittest/CCalendarComponentAdaptiveBucketingTest.cc

@@ -472,6 +472,18 @@ void CCalendarComponentAdaptiveBucketingTest::testPersist() {
     CPPUNIT_ASSERT_EQUAL(origXml, newXml);
 }
 
+void CCalendarComponentAdaptiveBucketingTest::testName() {
+    double decayRate{0.1};
+    double minimumBucketLength{1.0};
+
+    maths::CCalendarFeature feature{maths::CCalendarFeature::DAYS_SINCE_START_OF_MONTH, 0};
+    maths::CCalendarComponentAdaptiveBucketing bucketing{feature, decayRate, minimumBucketLength};
+
+    CPPUNIT_ASSERT_EQUAL(std::string("Calendar[") + std::to_string(decayRate) +
+                             "," + std::to_string(minimumBucketLength) + "]",
+                         bucketing.name());
+}
+
 CppUnit::Test* CCalendarComponentAdaptiveBucketingTest::suite() {
     CppUnit::TestSuite* suiteOfTests =
         new CppUnit::TestSuite("CCalendarComponentAdaptiveBucketingTest");
@@ -500,6 +512,9 @@ CppUnit::Test* CCalendarComponentAdaptiveBucketingTest::suite() {
     suiteOfTests->addTest(new CppUnit::TestCaller<CCalendarComponentAdaptiveBucketingTest>(
         "CCalendarComponentAdaptiveBucketingTest::testPersist",
         &CCalendarComponentAdaptiveBucketingTest::testPersist));
+    suiteOfTests->addTest(new CppUnit::TestCaller<CCalendarComponentAdaptiveBucketingTest>(
+        "CCalendarComponentAdaptiveBucketingTest::testName",
+        &CCalendarComponentAdaptiveBucketingTest::testName));
 
     return suiteOfTests;
 }

lib/maths/unittest/CCalendarComponentAdaptiveBucketingTest.h

@@ -21,6 +21,7 @@ class CCalendarComponentAdaptiveBucketingTest : public CppUnit::TestFixture {
     void testUnintialized();
     void testKnots();
     void testPersist();
+    void testName();
 
     static CppUnit::Test* suite();
 

lib/maths/unittest/CSeasonalComponentAdaptiveBucketingTest.cc

@@ -786,6 +786,18 @@ void CSeasonalComponentAdaptiveBucketingTest::testUpgrade() {
                          core::CContainerPrinter::print(restoredVariances));
 }
 
+void CSeasonalComponentAdaptiveBucketingTest::testName() {
+    double decayRate{0.1};
+    double minimumBucketLength{1.0};
+
+    maths::CDiurnalTime time(0, 0, core::constants::WEEK, core::constants::DAY);
+    maths::CSeasonalComponentAdaptiveBucketing bucketing(time, decayRate, minimumBucketLength);
+
+    CPPUNIT_ASSERT_EQUAL(std::string("Seasonal[") + std::to_string(decayRate) +
+                             "," + std::to_string(minimumBucketLength) + "]",
+                         bucketing.name());
+}
+
 CppUnit::Test* CSeasonalComponentAdaptiveBucketingTest::suite() {
     CppUnit::TestSuite* suiteOfTests =
         new CppUnit::TestSuite("CSeasonalComponentAdaptiveBucketingTest");
@@ -826,6 +838,9 @@ CppUnit::Test* CSeasonalComponentAdaptiveBucketingTest::suite() {
     suiteOfTests->addTest(new CppUnit::TestCaller<CSeasonalComponentAdaptiveBucketingTest>(
         "CSeasonalComponentAdaptiveBucketingTest::testUpgrade",
         &CSeasonalComponentAdaptiveBucketingTest::testUpgrade));
+    suiteOfTests->addTest(new CppUnit::TestCaller<CSeasonalComponentAdaptiveBucketingTest>(
+        "CSeasonalComponentAdaptiveBucketingTest::testName",
+        &CSeasonalComponentAdaptiveBucketingTest::testName));
 
     return suiteOfTests;
 }

lib/maths/unittest/CSeasonalComponentAdaptiveBucketingTest.h

@@ -23,6 +23,7 @@ class CSeasonalComponentAdaptiveBucketingTest : public CppUnit::TestFixture {
     void testSlope();
     void testPersist();
     void testUpgrade();
+    void testName();
 
     static CppUnit::Test* suite();
 };