[ML] Fix total feature importance unit tests (#1519)

We have an occasional failure in the total feature importance unit tests like this one. It happens while for some reason the mean value accumulator fails to produce correct results:

            {
                "feature_name": "c2",
                "classes": [
                    {
                        "class_name": "foo",
                        "importance": {
                            "mean_magnitude": 0,
                            "min": -3.288343526748277,
                            "max": 3.288343526748277
                        }
                    },
                    {
                        "class_name": "bar",
                        "importance": {
                            "mean_magnitude": 0,
                            "min": -3.288343526748277,
                            "max": 3.288343526748277
                        }
                    }
                ]
            }

In an attempt to fix this I changed the mean accumulator from using the eigen vector type to an explicit vector of doubles same as the min/max accumulator.

I keep the instrumentation of the unit tests for now so we have enough info if the tests continue to fail.

Author: valeriy42

include/api/CInferenceModelMetadata.h

@@ -50,16 +50,17 @@ class API_EXPORT CInferenceModelMetadata {
     void addToFeatureImportance(std::size_t i, const TVector& values);
 
 private:
-    using TMeanVarAccumulator = maths::CBasicStatistics::SSampleMeanVar<TVector>::TAccumulator;
+    using TMeanAccumulator =
+        std::vector<maths::CBasicStatistics::SSampleMean<double>::TAccumulator>;
     using TMinMaxAccumulator = std::vector<maths::CBasicStatistics::CMinMax<double>>;
-    using TSizeMeanVarAccumulatorUMap = std::unordered_map<std::size_t, TMeanVarAccumulator>;
+    using TSizeMeanAccumulatorUMap = std::unordered_map<std::size_t, TMeanAccumulator>;
     using TSizeMinMaxAccumulatorUMap = std::unordered_map<std::size_t, TMinMaxAccumulator>;
 
 private:
     void writeTotalFeatureImportance(TRapidJsonWriter& writer) const;
 
 private:
-    TSizeMeanVarAccumulatorUMap m_TotalShapValuesMeanVar;
+    TSizeMeanAccumulatorUMap m_TotalShapValuesMean;
     TSizeMinMaxAccumulatorUMap m_TotalShapValuesMinMax;
     TStrVec m_ColumnNames;
     TStrVec m_ClassValues;

lib/api/CInferenceModelMetadata.cc

@@ -5,6 +5,8 @@
  */
 #include <api/CInferenceModelMetadata.h>
 
+#include <cmath>
+
 namespace ml {
 namespace api {
 
@@ -15,7 +17,7 @@ void CInferenceModelMetadata::write(TRapidJsonWriter& writer) const {
 void CInferenceModelMetadata::writeTotalFeatureImportance(TRapidJsonWriter& writer) const {
     writer.Key(JSON_TOTAL_FEATURE_IMPORTANCE_TAG);
     writer.StartArray();
-    for (const auto& item : m_TotalShapValuesMeanVar) {
+    for (const auto& item : m_TotalShapValuesMean) {
         writer.StartObject();
         writer.Key(JSON_FEATURE_NAME_TAG);
         writer.String(m_ColumnNames[item.first]);
@@ -104,14 +106,15 @@ void CInferenceModelMetadata::predictionFieldTypeResolverWriter(
 }
 
 void CInferenceModelMetadata::addToFeatureImportance(std::size_t i, const TVector& values) {
-    m_TotalShapValuesMeanVar
-        .emplace(std::make_pair(i, TVector::Zero(values.size())))
-        .first->second.add(values.cwiseAbs());
+    auto& meanVector = m_TotalShapValuesMean
+                           .emplace(std::make_pair(i, TMeanAccumulator(values.size())))
+                           .first->second;
     auto& minMaxVector =
         m_TotalShapValuesMinMax
             .emplace(std::make_pair(i, TMinMaxAccumulator(values.size())))
             .first->second;
     for (std::size_t j = 0; j < minMaxVector.size(); ++j) {
+        meanVector[j].add(std::fabs(values[j]));
         minMaxVector[j].add(values[j]);
     }
 }