[ML] Boosted tree tidy ups (elastic#1155)

Author: tveasey

include/maths/CBoostedTreeLeafNodeStatistics.h

@@ -506,7 +506,6 @@ class MATHS_EXPORT CBoostedTreeLeafNodeStatistics final {
     };
 
 private:
-    void maybeRecoverMemory();
     void computeAggregateLossDerivatives(std::size_t numberThreads,
                                          const core::CDataFrame& frame,
                                          const CDataFrameCategoryEncoder& encoder);

lib/maths/CBoostedTreeImpl.cc

@@ -1005,12 +1005,13 @@ void CBoostedTreeImpl::refreshPredictionsAndLossDerivatives(core::CDataFrame& fr
             core::bindRetrievableState(
                 [&](TArgMinLossVec& leafValues_, TRowItr beginRows, TRowItr endRows) {
                     std::size_t numberLossParameters{m_Loss->numberParameters()};
-                    for (auto row = beginRows; row != endRows; ++row) {
-                        auto prediction = readPrediction(*row, m_ExtraColumns,
+                    for (auto row_ = beginRows; row_ != endRows; ++row_) {
+                        auto row = *row_;
+                        auto prediction = readPrediction(row, m_ExtraColumns,
                                                          numberLossParameters);
-                        double actual{readActual(*row, m_DependentVariable)};
-                        double weight{readExampleWeight(*row, m_ExtraColumns)};
-                        leafValues_[root(tree).leafIndex(m_Encoder->encode(*row), tree)]
+                        double actual{readActual(row, m_DependentVariable)};
+                        double weight{readExampleWeight(row, m_ExtraColumns)};
+                        leafValues_[root(tree).leafIndex(m_Encoder->encode(row), tree)]
                             .add(prediction, actual, weight);
                     }
                 },
@@ -1038,13 +1039,14 @@ void CBoostedTreeImpl::refreshPredictionsAndLossDerivatives(core::CDataFrame& fr
         m_NumberThreads, 0, frame.numberRows(),
         [&](TRowItr beginRows, TRowItr endRows) {
             std::size_t numberLossParameters{m_Loss->numberParameters()};
-            for (auto row = beginRows; row != endRows; ++row) {
-                auto prediction = readPrediction(*row, m_ExtraColumns, numberLossParameters);
-                double actual{readActual(*row, m_DependentVariable)};
-                double weight{readExampleWeight(*row, m_ExtraColumns)};
-                prediction += root(tree).value(m_Encoder->encode(*row), tree);
-                writeLossGradient(*row, m_ExtraColumns, *m_Loss, prediction, actual, weight);
-                writeLossCurvature(*row, m_ExtraColumns, *m_Loss, prediction, actual, weight);
+            for (auto row_ = beginRows; row_ != endRows; ++row_) {
+                auto row = *row_;
+                auto prediction = readPrediction(row, m_ExtraColumns, numberLossParameters);
+                double actual{readActual(row, m_DependentVariable)};
+                double weight{readExampleWeight(row, m_ExtraColumns)};
+                prediction += root(tree).value(m_Encoder->encode(row), tree);
+                writeLossGradient(row, m_ExtraColumns, *m_Loss, prediction, actual, weight);
+                writeLossCurvature(row, m_ExtraColumns, *m_Loss, prediction, actual, weight);
             }
         },
         &updateRowMask);

lib/maths/CBoostedTreeLeafNodeStatistics.cc

@@ -104,8 +104,6 @@ CBoostedTreeLeafNodeStatistics::split(std::size_t leftChildId,
         auto rightChild = std::make_shared<CBoostedTreeLeafNodeStatistics>(
             rightChildId, std::move(*this), *leftChild, regularization,
             featureBag, std::move(rightChildRowMask));
-        leftChild->maybeRecoverMemory();
-        rightChild->maybeRecoverMemory();
 
         return std::make_pair(leftChild, rightChild);
     }
@@ -119,8 +117,6 @@ CBoostedTreeLeafNodeStatistics::split(std::size_t leftChildId,
     auto leftChild = std::make_shared<CBoostedTreeLeafNodeStatistics>(
         leftChildId, std::move(*this), *rightChild, regularization, featureBag,
         std::move(leftChildRowMask));
-    leftChild->maybeRecoverMemory();
-    rightChild->maybeRecoverMemory();
 
     return std::make_pair(leftChild, rightChild);
 }
@@ -176,13 +172,6 @@ CBoostedTreeLeafNodeStatistics::estimateMemoryUsage(std::size_t numberRows,
     return sizeof(CBoostedTreeLeafNodeStatistics) + rowMaskSize + splitsDerivativesSize;
 }
 
-void CBoostedTreeLeafNodeStatistics::maybeRecoverMemory() {
-    if (this->gain() <= 0.0) {
-        m_RowMask = core::CPackedBitVector{};
-        m_Derivatives = CSplitsDerivatives{};
-    }
-}
-
 void CBoostedTreeLeafNodeStatistics::computeAggregateLossDerivatives(
     std::size_t numberThreads,
     const core::CDataFrame& frame,

lib/maths/CBoostedTreeLoss.cc

@@ -482,8 +482,6 @@ CArgMinMsleImpl::TObjective CArgMinMsleImpl::objective() const {
     return [this](double logWeight) {
 
         double weight{std::exp(logWeight)};
-        double loss{0.0};
-        double totalCount{0.0};
         if (this->bucketWidth().first == 0.0) {
             // prediction is constant
             double expPrediction{m_ExpPredictionMinMax.max()};
@@ -495,6 +493,8 @@ CArgMinMsleImpl::TObjective CArgMinMsleImpl::objective() const {
                         CTools::pow2(logPrediction)};
             return loss + this->lambda() * CTools::pow2(weight);
         } else {
+            double loss{0.0};
+            double totalCount{0.0};
             for (const auto& bucketPrediction : m_Buckets) {
                 for (const auto& bucketActual : bucketPrediction) {
                     double count{CBasicStatistics::count(bucketActual)};