[Predator] Move ``SingleFeatureScore`` to LLM.

appengine/findit/crash/loglinear/changelist_classifier.py

 # Copyright 2016 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import logging
 import math
 
 from common.chrome_dependency_fetcher import ChromeDependencyFetcher
 from crash import changelist_classifier
 from crash.loglinear.changelist_features import min_distance
@@ skipping 31 matching lines @@
     feature_function = ToFeatureFunction([
         top_frame_index.TopFrameIndexFeature(top_n_frames),
         min_distance.MinDistanceFeature(),
     ])
 
     weight_list = [
         weights['TopFrameIndex'],
         weights['MinDistance'],
     ]
 
-    self._model = UnnormalizedLogLinearModel(feature_function, weight_list)
-
-    # TODO(crbug.com/674262): remove the need for storing these weights.
-    self._weights = weights
+    self._model = UnnormalizedLogLinearModel(feature_function,
+                                             weight_list, weights)

[wrengr, 2017/01/09 19:31:51]

we shouldn't have to pass both the weight dict and list; the dict should be
sufficient.

 
   # TODO(crbug.com/673964): something better for detecting "close to log(0)".
   def _LogZeroish(self, x):
     """Determine whether a float is close enough to log(0).
 
     If a ``FeatureValue`` has a (log-domain) score of -inf for a given
     ``Suspect``, then that suspect has zero probability of being the
     culprit. We want to filter these suspects out, to clean up the
     output of classification; so this method encapsulates the logic of
     that check.
 
     Args:
       x (float): the float to check
 
     Returns:
       ``True`` if ``x`` is close enough to log(0); else ``False``.
     """
     return x < 0 and math.isinf(x)
 
-  def _SingleFeatureScore(self, feature_value):
-    """Returns the score (aka weighted value) of a ``FeatureValue``.
-
-    This function assumes the report's stacktrace has already had any necessary
-    preprocessing (like filtering or truncating) applied.
-
-    Args:
-      feature_value (FeatureValue): the feature value to check.
-
-    Returns:
-      The score of the feature value.
-    """
-    return feature_value.value * self._weights.get(feature_value.name, 0.)
-
   def __call__(self, report):
     """Finds changelists suspected of being responsible for the crash report.
 
     Args:
       report (CrashReport): the report to be analyzed.
 
     Returns:
       List of ``Suspect``s, sorted by probability from highest to lowest.
     """
     if not report.regression_range:
@@ skipping 82 matching lines @@
       sorted by feature name, just to ensure that it comes out in some
       canonical order.
 
       At present, the float is the log-domain score of the feature
       value. However, this isn't the best thing for UX reasons. In the
       future it might be replaced by the normal-domain score, or by
       the probability.
     """
     formatted_reasons = []
     for feature in features:
-      feature_score = self._SingleFeatureScore(feature)
+      feature_score = self._model.SingleFeatureScore(feature)
       if self._LogZeroish(feature_score): # pragma: no cover
         logging.debug('Discarding reasons from feature %s'
             ' because it has zero probability' % feature.name)
         continue
 
       formatted_reasons.append((feature.name, feature_score, feature.reason))
 
     return sorted(formatted_reasons,
         key=lambda formatted_reason: formatted_reason[0])
 
   def AggregateChangedFiles(self, features):
     """Merge multiple``FeatureValue.changed_files`` lists into one.
 
     Args:
       features (list of FeatureValue): the values whose ``changed_files``
         lists should be aggregated.
 
     Returns:
       A list of ``ChangedFile`` objects sorted by file name. The sorting
       is not essential, but is provided to ease testing by ensuring the
       output is in some canonical order.
 
     Raises:
       ``ValueError`` if any file name is given inconsistent ``blame_url``s.
     """
     all_changed_files = {}
     for feature in features:
-      if self._LogZeroish(self._SingleFeatureScore(feature)): # pragma: no cover
+      if self._LogZeroish(
+          self._model.SingleFeatureScore(feature)): # pragma: no cover
         logging.debug('Discarding changed files from feature %s'
             ' because it has zero probability' % feature.name)
         continue
 
       for changed_file in feature.changed_files or []:
         accumulated_changed_file = all_changed_files.get(changed_file.name)
         if accumulated_changed_file is None:
           all_changed_files[changed_file.name] = changed_file
           continue
 
         assert accumulated_changed_file.blame_url == changed_file.blame_url, (
             ValueError('Blame URLs do not match: %s != %s'
                 % (accumulated_changed_file.blame_url, changed_file.blame_url)))
         accumulated_changed_file.reasons.extend(changed_file.reasons or [])
 
     return sorted(all_changed_files.values(),
         key=lambda changed_file: changed_file.name)