[ML] Remove old per-partition normalization code

edsavage · edsavage · commit c03ac54fbf82 · 2018-08-16T16:34:05.000+01:00
Per-partition normalization is an old, undocumented feature that was never used by clients. It has been superseded by per-partition maximum scoring (see #32748). This PR removes the now redundant code. Relates elastic/elasticsearch#32816
diff --git a/bin/autodetect/CCmdLineParser.cc b/bin/autodetect/CCmdLineParser.cc
@@ -53,7 +53,6 @@ bool CCmdLineParser::parse(int argc,
                            std::size_t& bucketResultsDelay,
                            bool& multivariateByFields,
                            std::string& multipleBucketspans,
-                           bool& perPartitionNormalization,
                            TStrVec& clauseTokens) {
     try {
         boost::program_options::options_description desc(DESCRIPTION);
@@ -119,8 +118,6 @@ bool CCmdLineParser::parse(int argc,
                         "Optional flag to enable multi-variate analysis of correlated by fields")
             ("multipleBucketspans",  boost::program_options::value<std::string>(),
                         "Optional comma-separated list of additional bucketspans - must be direct multiples of the main bucketspan")
-            ("perPartitionNormalization",
-                        "Optional flag to enable per partition normalization")
         ;
         // clang-format on
 
@@ -237,9 +234,6 @@ bool CCmdLineParser::parse(int argc,
         if (vm.count("multipleBucketspans") > 0) {
             multipleBucketspans = vm["multipleBucketspans"].as<std::string>();
         }
-        if (vm.count("perPartitionNormalization") > 0) {
-            perPartitionNormalization = true;
-        }
 
         boost::program_options::collect_unrecognized(
             parsed.options, boost::program_options::include_positional)
diff --git a/bin/autodetect/CCmdLineParser.h b/bin/autodetect/CCmdLineParser.h
@@ -65,7 +65,6 @@ class CCmdLineParser {
                       std::size_t& bucketResultsDelay,
                       bool& multivariateByFields,
                       std::string& multipleBucketspans,
-                      bool& perPartitionNormalization,
                       TStrVec& clauseTokens);
 
 private:
diff --git a/bin/autodetect/Main.cc b/bin/autodetect/Main.cc
@@ -89,18 +89,16 @@ int main(int argc, char** argv) {
     std::size_t bucketResultsDelay(0);
     bool multivariateByFields(false);
     std::string multipleBucketspans;
-    bool perPartitionNormalization(false);
     TStrVec clauseTokens;
     if (ml::autodetect::CCmdLineParser::parse(
             argc, argv, limitConfigFile, modelConfigFile, fieldConfigFile,
             modelPlotConfigFile, jobId, logProperties, logPipe, bucketSpan, latency,
             summaryCountFieldName, delimiter, lengthEncodedInput, timeField,
             timeFormat, quantilesStateFile, deleteStateFiles, persistInterval,
             maxQuantileInterval, inputFileName, isInputFileNamedPipe, outputFileName,
-            isOutputFileNamedPipe, restoreFileName, isRestoreFileNamedPipe,
-            persistFileName, isPersistFileNamedPipe, maxAnomalyRecords, memoryUsage,
-            bucketResultsDelay, multivariateByFields, multipleBucketspans,
-            perPartitionNormalization, clauseTokens) == false) {
+            isOutputFileNamedPipe, restoreFileName, isRestoreFileNamedPipe, persistFileName,
+            isPersistFileNamedPipe, maxAnomalyRecords, memoryUsage, bucketResultsDelay,
+            multivariateByFields, multipleBucketspans, clauseTokens) == false) {
         return EXIT_FAILURE;
     }
 
@@ -148,7 +146,6 @@ int main(int argc, char** argv) {
         ml::model::CAnomalyDetectorModelConfig::defaultConfig(
             bucketSpan, summaryMode, summaryCountFieldName, latency,
             bucketResultsDelay, multivariateByFields, multipleBucketspans);
-    modelConfig.perPartitionNormalization(perPartitionNormalization);
     modelConfig.detectionRules(ml::model::CAnomalyDetectorModelConfig::TIntDetectionRuleVecUMapCRef(
         fieldConfig.detectionRules()));
     modelConfig.scheduledEvents(ml::model::CAnomalyDetectorModelConfig::TStrDetectionRulePrVecCRef(
diff --git a/bin/normalize/CCmdLineParser.cc b/bin/normalize/CCmdLineParser.cc
@@ -30,8 +30,7 @@ bool CCmdLineParser::parse(int argc,
                            bool& isOutputFileNamedPipe,
                            std::string& quantilesState,
                            bool& deleteStateFiles,
-                           bool& writeCsv,
-                           bool& perPartitionNormalization) {
+                           bool& writeCsv) {
     try {
         boost::program_options::options_description desc(DESCRIPTION);
         // clang-format off
@@ -60,8 +59,6 @@ bool CCmdLineParser::parse(int argc,
                         "If this flag is set then delete the normalizer state files once they have been read")
             ("writeCsv",
                         "Write the results in CSV format (default is lineified JSON)")
-            ("perPartitionNormalization",
-                        "Optional flag to enable per partition normalization")
         ;
         // clang-format on
 
@@ -114,9 +111,6 @@ bool CCmdLineParser::parse(int argc,
         if (vm.count("writeCsv") > 0) {
             writeCsv = true;
         }
-        if (vm.count("perPartitionNormalization") > 0) {
-            perPartitionNormalization = true;
-        }
     } catch (std::exception& e) {
         std::cerr << "Error processing command line: " << e.what() << std::endl;
         return false;
diff --git a/bin/normalize/CCmdLineParser.h b/bin/normalize/CCmdLineParser.h
@@ -43,8 +43,7 @@ class CCmdLineParser {
                       bool& isOutputFileNamedPipe,
                       std::string& quantilesState,
                       bool& deleteStateFiles,
-                      bool& writeCsv,
-                      bool& perPartitionNormalization);
+                      bool& writeCsv);
 
 private:
     static const std::string DESCRIPTION;
diff --git a/bin/normalize/Main.cc b/bin/normalize/Main.cc
@@ -54,12 +54,10 @@ int main(int argc, char** argv) {
     std::string quantilesStateFile;
     bool deleteStateFiles(false);
     bool writeCsv(false);
-    bool perPartitionNormalization(false);
     if (ml::normalize::CCmdLineParser::parse(
-            argc, argv, modelConfigFile, logProperties, logPipe, bucketSpan,
-            lengthEncodedInput, inputFileName, isInputFileNamedPipe,
-            outputFileName, isOutputFileNamedPipe, quantilesStateFile,
-            deleteStateFiles, writeCsv, perPartitionNormalization) == false) {
+            argc, argv, modelConfigFile, logProperties, logPipe, bucketSpan, lengthEncodedInput,
+            inputFileName, isInputFileNamedPipe, outputFileName, isOutputFileNamedPipe,
+            quantilesStateFile, deleteStateFiles, writeCsv) == false) {
         return EXIT_FAILURE;
     }
 
@@ -93,7 +91,6 @@ int main(int argc, char** argv) {
         LOG_FATAL(<< "Ml model config file '" << modelConfigFile << "' could not be loaded");
         return EXIT_FAILURE;
     }
-    modelConfig.perPartitionNormalization(perPartitionNormalization);
 
     // There's a choice of input and output formats for the numbers to be normalised
     using TInputParserUPtr = std::unique_ptr<ml::api::CInputParser>;
diff --git a/include/api/CHierarchicalResultsWriter.h b/include/api/CHierarchicalResultsWriter.h
@@ -51,12 +51,7 @@ class API_EXPORT CHierarchicalResultsWriter : public model::CHierarchicalResults
     using TStr1Vec = core::CSmallVector<std::string, 1>;
 
 public:
-    enum EResultType {
-        E_SimpleCountResult,
-        E_PopulationResult,
-        E_PartitionResult,
-        E_Result
-    };
+    enum EResultType { E_SimpleCountResult, E_PopulationResult, E_Result };
     //! Type which wraps up the results of anomaly detection.
     struct API_EXPORT SResults {
         //! Construct for population results
@@ -168,9 +163,6 @@ class API_EXPORT CHierarchicalResultsWriter : public model::CHierarchicalResults
     //! pivot.
     void writePivotResult(const model::CHierarchicalResults& results, const TNode& node);
 
-    //! Write partition result if \p node is a partition level result
-    void writePartitionResult(const model::CHierarchicalResults& results, const TNode& node);
-
     //! Write out a simple count result if \p node is simple
     //! count.
     void writeSimpleCountResult(const TNode& node);
diff --git a/include/api/CJsonOutputWriter.h b/include/api/CJsonOutputWriter.h
@@ -162,9 +162,6 @@ class API_EXPORT CJsonOutputWriter : public COutputHandler {
         // when the number to write is limited
         double s_LowestBucketInfluencerScore;
 
-        //! Partition scores
-        TDocumentWeakPtrVec s_PartitionScoreDocuments;
-
         //! scheduled event descriptions
         TStr1Vec s_ScheduledEventDescriptions;
     };
@@ -304,10 +301,6 @@ class API_EXPORT CJsonOutputWriter : public COutputHandler {
     void addInfluences(const CHierarchicalResultsWriter::TStoredStringPtrStoredStringPtrPrDoublePrVec& influenceResults,
                        TDocumentWeakPtr weakDoc);
 
-    //! Write partition score & probability
-    void addPartitionScores(const CHierarchicalResultsWriter::TResults& results,
-                            TDocumentWeakPtr weakDoc);
-
 private:
     //! The job ID
     std::string m_JobId;
diff --git a/include/api/CResultNormalizer.h b/include/api/CResultNormalizer.h
@@ -93,15 +93,6 @@ class API_EXPORT CResultNormalizer {
                          std::string& valueFieldName,
                          double& probability);
 
-    bool parseDataFields(const TStrStrUMap& dataRowFields,
-                         std::string& level,
-                         std::string& partition,
-                         std::string& partitionValue,
-                         std::string& person,
-                         std::string& function,
-                         std::string& valueFieldName,
-                         double& probability);
-
     template<typename T>
     bool parseDataField(const TStrStrUMap& dataRowFields,
                         const std::string& fieldName,
diff --git a/include/model/CAnomalyDetectorModelConfig.h b/include/model/CAnomalyDetectorModelConfig.h
@@ -424,12 +424,6 @@ class MODEL_EXPORT CAnomalyDetectorModelConfig {
     const TDoubleDoublePrVec& normalizedScoreKnotPoints() const;
     //@}
 
-    //! Check if we should create one normalizer per partition field value.
-    bool perPartitionNormalization() const;
-
-    //! Set whether we should create one normalizer per partition field value.
-    void perPartitionNormalization(bool value);
-
     //! Sets the reference to the detection rules map
     void detectionRules(TIntDetectionRuleVecUMapCRef detectionRules);
 
@@ -500,9 +494,6 @@ class MODEL_EXPORT CAnomalyDetectorModelConfig {
     //! and the normalized anomaly score with these knot points.
     //! \see DEFAULT_NORMALIZED_SCORE_KNOT_POINTS for details.
     TDoubleDoublePrVec m_NormalizedScoreKnotPoints;
-
-    //! If true then create one normalizer per partition field value.
-    bool m_PerPartitionNormalisation;
     //@}
 
     //! A reference to the map containing detection rules per
diff --git a/lib/api/CHierarchicalResultsWriter.cc b/lib/api/CHierarchicalResultsWriter.cc
@@ -126,7 +126,6 @@ void CHierarchicalResultsWriter::visit(const model::CHierarchicalResults& result
     } else {
         this->writePopulationResult(results, node);
         this->writeIndividualResult(results, node);
-        this->writePartitionResult(results, node);
         this->writeSimpleCountResult(node);
     }
 }
@@ -258,34 +257,6 @@ void CHierarchicalResultsWriter::writeIndividualResult(const model::CHierarchica
         node.s_Spec.s_Detector, node.s_BucketLength, EMPTY_STRING_LIST));
 }
 
-void CHierarchicalResultsWriter::writePartitionResult(const model::CHierarchicalResults& results,
-                                                      const TNode& node) {
-    if (!m_ModelConfig.perPartitionNormalization() || this->isSimpleCount(node) ||
-        this->isPopulation(node) || !this->isPartition(node) ||
-        !this->shouldWriteResult(m_Limits, results, node, false)) {
-        return;
-    }
-
-    model_t::EFeature feature =
-        node.s_AnnotatedProbability.s_AttributeProbabilities.empty()
-            ? model_t::E_IndividualCountByBucketAndPerson
-            : node.s_AnnotatedProbability.s_AttributeProbabilities[0].s_Feature;
-
-    TDouble1Vec emptyDoubleVec;
-
-    m_ResultWriterFunc(TResults(
-        E_PartitionResult, *node.s_Spec.s_PartitionFieldName,
-        *node.s_Spec.s_PartitionFieldValue, *node.s_Spec.s_ByFieldName,
-        *node.s_Spec.s_PersonFieldValue, EMPTY_STRING, node.s_BucketStartTime,
-        *node.s_Spec.s_FunctionName, model_t::outputFunctionName(feature),
-        node.s_AnnotatedProbability.s_BaselineBucketCount,
-        node.s_AnnotatedProbability.s_CurrentBucketCount, emptyDoubleVec, emptyDoubleVec,
-        node.s_RawAnomalyScore, node.s_NormalizedAnomalyScore, node.probability(),
-        *node.s_Spec.s_ValueFieldName, node.s_AnnotatedProbability.s_Influences,
-        node.s_Spec.s_UseNull, model::function_t::isMetric(node.s_Spec.s_Function),
-        node.s_Spec.s_Detector, node.s_BucketLength, EMPTY_STRING_LIST));
-}
-
 void CHierarchicalResultsWriter::writePivotResult(const model::CHierarchicalResults& results,
                                                   const TNode& node) {
     if (this->isSimpleCount(node) ||
diff --git a/lib/api/CJsonOutputWriter.cc b/lib/api/CJsonOutputWriter.cc
@@ -70,7 +70,6 @@ const std::string EXAMPLES("examples");
 const std::string BUCKET_SPAN("bucket_span");
 const std::string PROCESSING_TIME("processing_time_ms");
 const std::string TIME_INFLUENCER("bucket_time");
-const std::string PARTITION_SCORES("partition_scores");
 const std::string SCHEDULED_EVENTS("scheduled_events");
 const std::string QUANTILES("quantiles");
 
@@ -191,14 +190,6 @@ bool CJsonOutputWriter::acceptResult(const CHierarchicalResultsWriter::TResults&
         return true;
     }
 
-    if (results.s_ResultType == CHierarchicalResultsWriter::E_PartitionResult) {
-        TDocumentWeakPtr partitionDoc = m_Writer.makeStorableDoc();
-        this->addPartitionScores(results, partitionDoc);
-        bucketData.s_PartitionScoreDocuments.push_back(partitionDoc);
-
-        return true;
-    }
-
     ++bucketData.s_RecordCount;
 
     TDocumentWeakPtrIntPrVec& detectorDocumentsToWrite = bucketData.s_DocumentsToWrite;
@@ -513,26 +504,6 @@ void CJsonOutputWriter::writeBucket(bool isInterim,
         m_Writer.EndArray();
     }
 
-    if (!bucketData.s_PartitionScoreDocuments.empty()) {
-        // Write the array of partition-anonaly score pairs
-        m_Writer.String(PARTITION_SCORES);
-        m_Writer.StartArray();
-        for (TDocumentWeakPtrVecItr partitionScoresIter =
-                 bucketData.s_PartitionScoreDocuments.begin();
-             partitionScoresIter != bucketData.s_PartitionScoreDocuments.end();
-             ++partitionScoresIter) {
-            TDocumentWeakPtr weakDoc = *partitionScoresIter;
-            TDocumentPtr docPtr = weakDoc.lock();
-            if (!docPtr) {
-                LOG_ERROR(<< "Inconsistent program state. JSON document unavailable.");
-                continue;
-            }
-
-            m_Writer.write(*docPtr);
-        }
-        m_Writer.EndArray();
-    }
-
     m_Writer.String(PROCESSING_TIME);
     m_Writer.Uint64(bucketProcessingTime);
 
@@ -816,24 +787,6 @@ void CJsonOutputWriter::addInfluencerFields(bool isBucketInfluencer,
     }
 }
 
-void CJsonOutputWriter::addPartitionScores(const CHierarchicalResultsWriter::TResults& results,
-                                           TDocumentWeakPtr weakDoc) {
-    TDocumentPtr docPtr = weakDoc.lock();
-    if (!docPtr) {
-        LOG_ERROR(<< "Inconsistent program state. JSON document unavailable.");
-        return;
-    }
-
-    m_Writer.addDoubleFieldToObj(PROBABILITY, results.s_Probability, *docPtr);
-    m_Writer.addStringFieldCopyToObj(PARTITION_FIELD_NAME,
-                                     results.s_PartitionFieldName, *docPtr);
-    m_Writer.addStringFieldCopyToObj(PARTITION_FIELD_VALUE,
-                                     results.s_PartitionFieldValue, *docPtr, true);
-    m_Writer.addDoubleFieldToObj(INITIAL_RECORD_SCORE,
-                                 results.s_NormalizedAnomalyScore, *docPtr);
-    m_Writer.addDoubleFieldToObj(RECORD_SCORE, results.s_NormalizedAnomalyScore, *docPtr);
-}
-
 void CJsonOutputWriter::limitNumberRecords(size_t count) {
     m_RecordOutputLimit = count;
 }
diff --git a/lib/api/CResultNormalizer.cc b/lib/api/CResultNormalizer.cc
@@ -75,18 +75,8 @@ bool CResultNormalizer::handleRecord(const TStrStrUMap& dataRowFields) {
     std::string valueFieldName;
     double probability(0.0);
 
-    bool isValidRecord(false);
-    if (m_ModelConfig.perPartitionNormalization()) {
-        isValidRecord = parseDataFields(dataRowFields, level, partition, partitionValue,
-                                        person, function, valueFieldName, probability);
-    } else {
-        isValidRecord = parseDataFields(dataRowFields, level, partition, person,
-                                        function, valueFieldName, probability);
-    }
-
-    std::string partitionKey = m_ModelConfig.perPartitionNormalization()
-                                   ? partition + partitionValue
-                                   : partition;
+    bool isValidRecord = parseDataFields(dataRowFields, level, partition, person,
+                                         function, valueFieldName, probability);
 
     if (isValidRecord) {
         const model::CAnomalyScore::CNormalizer* levelNormalizer = nullptr;
@@ -96,10 +86,10 @@ bool CResultNormalizer::handleRecord(const TStrStrUMap& dataRowFields) {
         if (level == ROOT_LEVEL) {
             levelNormalizer = &m_Normalizer.bucketNormalizer();
         } else if (level == LEAF_LEVEL) {
-            levelNormalizer = m_Normalizer.leafNormalizer(partitionKey, person,
+            levelNormalizer = m_Normalizer.leafNormalizer(partition, person,
                                                           function, valueFieldName);
         } else if (level == PARTITION_LEVEL) {
-            levelNormalizer = m_Normalizer.partitionNormalizer(partitionKey);
+            levelNormalizer = m_Normalizer.partitionNormalizer(partition);
         } else if (level == BUCKET_INFLUENCER_LEVEL) {
             levelNormalizer = m_Normalizer.influencerBucketNormalizer(person);
         } else if (level == INFLUENCER_LEVEL) {
@@ -148,22 +138,5 @@ bool CResultNormalizer::parseDataFields(const TStrStrUMap& dataRowFields,
            this->parseDataField(dataRowFields, VALUE_FIELD_NAME, valueFieldName) &&
            this->parseDataField(dataRowFields, PROBABILITY_NAME, probability);
 }
-
-bool CResultNormalizer::parseDataFields(const TStrStrUMap& dataRowFields,
-                                        std::string& level,
-                                        std::string& partition,
-                                        std::string& partitionValue,
-                                        std::string& person,
-                                        std::string& function,
-                                        std::string& valueFieldName,
-                                        double& probability) {
-    return this->parseDataField(dataRowFields, LEVEL, level) &&
-           this->parseDataField(dataRowFields, PARTITION_FIELD_NAME, partition) &&
-           this->parseDataField(dataRowFields, PARTITION_FIELD_VALUE, partitionValue) &&
-           this->parseDataField(dataRowFields, PERSON_FIELD_NAME, person) &&
-           this->parseDataField(dataRowFields, FUNCTION_NAME, function) &&
-           this->parseDataField(dataRowFields, VALUE_FIELD_NAME, valueFieldName) &&
-           this->parseDataField(dataRowFields, PROBABILITY_NAME, probability);
-}
 }
 }
diff --git a/lib/api/unittest/CJsonOutputWriterTest.cc b/lib/api/unittest/CJsonOutputWriterTest.cc
diff --git a/lib/api/unittest/CJsonOutputWriterTest.h b/lib/api/unittest/CJsonOutputWriterTest.h
diff --git a/lib/model/CAnomalyDetectorModelConfig.cc b/lib/model/CAnomalyDetectorModelConfig.cc
diff --git a/lib/model/CHierarchicalResultsNormalizer.cc b/lib/model/CHierarchicalResultsNormalizer.cc
diff --git a/lib/model/unittest/CAnomalyDetectorModelConfigTest.cc b/lib/model/unittest/CAnomalyDetectorModelConfigTest.cc
