Change "percentage change" function and add comments/test for it.

tools/bisect-perf-regression.py

 #!/usr/bin/env python
 # Copyright (c) 2013 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.
 
 """Performance Test Bisect Tool
 
 This script bisects a series of changelists using binary search. It starts at
 a bad revision where a performance metric has regressed, and asks for a last
 known-good revision. It will then binary search across this revision range by
@@ skipping 169 matching lines @@
 def _AddAdditionalDepotInfo(depot_info):
   """Adds additional depot info to the global depot variables."""
   global DEPOT_DEPS_NAME
   global DEPOT_NAMES
   DEPOT_DEPS_NAME = dict(DEPOT_DEPS_NAME.items() +
       depot_info.items())
   DEPOT_NAMES = DEPOT_DEPS_NAME.keys()
 
 
 def CalculateTruncatedMean(data_set, truncate_percent):
   """Calculates the truncated mean of a set of values.

[qyearsley, 2014/04/17 18:47:59]

I just did a quick search, and it appears that calculation of "truncated mean"
(or "trimmed mean") usually doesn't involve; usually the X% trimmed mean is
calculated by just excluding the top X% and bottom X% and taking the mean of the
rest of the values.

Reference:
http://www.d.umn.edu/~yqi/stat3611/trimmed.pdf
http://voices.yahoo.com/statistics-101-trimmed-mean-median-4152059.html
http://www.statistics.com/glossary&term_id=866

However, calculation of the "interquartile mean" may involve weighting two of
the values.
http://en.wikipedia.org/wiki/Interquartile_mean

Any thoughts? I'm thinking of replacing this with the simplified truncated mean
function, which doesn't weight anything.

[shatch, 2014/04/17 19:46:17]

I don't have a strong opinion on it, so go ahead with whatever you feel is best.

http://en.wikipedia.org/wiki/Truncated_mean

This does define the truncated/trimmed mean as using weighted values.
Additionally, according to the wikipedia entry you linked, the interquartile
mean is just the truncated mean with specific values (in this case, 25%).

On 2014/04/17 18:47:59, qyearsley wrote:

[qyearsley, 2014/04/17 20:54:05]

Ah, right! I overlooked that.

I think that the current truncated mean function is fine, I just had a bit of
trouble understanding it.

 
   Note that this isn't just the mean of the set of values with the highest
   and lowest values discarded; the non-discarded values are also weighted
   differently depending how many values are discarded.
 
   Args:
     data_set: Non-empty list of values.
     truncate_percent: The % from the upper and lower portions of the data set
         to discard, expressed as a value in [0, 1].
 
@@ skipping 83 matching lines @@
 
   mean = CalculateMean(values)
   differences_from_mean = [float(x) - mean for x in values]
   squared_differences = [float(x * x) for x in differences_from_mean]
   variance = sum(squared_differences) / (len(values) - 1)
   std_dev = math.sqrt(variance)
 
   return std_dev
 
 
+def CalculateRelativeChange(before, after):
+  """Returns the relative change of before and after, relative to before.
+
+  There are several different ways to define relative difference between
+  two numbers; sometimes it is defined as relative to the smaller number,

[qyearsley, 2014/04/17 18:47:59]

The previous version was calculating the difference relative to the smaller
number.

It was calculating: 
   (big/small) - 1.0
 = (big/small) - (small/small)
 = (big - small) / small

[shatch, 2014/04/17 19:46:17]

Does this change make it match the perf dashboard?

On 2014/04/17 18:47:59, qyearsley wrote:

[shatch, 2014/04/17 19:47:19]

Doh! This is answered in your CL description :)

On 2014/04/17 19:46:17, shatch wrote:

+  or to the mean of the two numbers. This version returns the difference
+  relative to the first of the two numbers.
+
+  Args:
+    before: A number representing an earlier value.
+    after: Another number, representing a later value.
+
+  Returns:
+    A non-negative floating point number; 0.1 represents a 10% change.
+  """
+  if before == 0:
+    return float('nan')
+  difference = math.fabs(after - before)
+  return math.fabs(difference / before)
+
+
 def CalculatePooledStandardError(work_sets):
   numerator = 0.0
   denominator1 = 0.0
   denominator2 = 0.0
 
   for current_set in work_sets:
     std_dev = CalculateStandardDeviation(current_set)
     numerator += (len(current_set) - 1) * std_dev ** 2
     denominator1 += len(current_set) - 1
     denominator2 += 1.0 / len(current_set)
@@ skipping 2826 matching lines @@
           working_means.append(revision_data_sorted[i][1]['value']['values'])
 
       # Flatten the lists to calculate mean of all values.
       working_mean = sum(working_means, [])
       broken_mean = sum(broken_means, [])
 
       # Calculate the approximate size of the regression
       mean_of_bad_runs = CalculateMean(broken_mean)
       mean_of_good_runs = CalculateMean(working_mean)
 
-      regression_size = math.fabs(max(mean_of_good_runs, mean_of_bad_runs) /
-          max(0.0001, min(mean_of_good_runs, mean_of_bad_runs))) * 100.0 - 100.0
+      regression_size = 100 * CalculateRelativeChange(mean_of_good_runs,
+                                                      mean_of_bad_runs)
 
       regression_std_err = math.fabs(CalculatePooledStandardError(
           [working_mean, broken_mean]) /
           max(0.0001, min(mean_of_good_runs, mean_of_bad_runs))) * 100.0
 
       # Give a "confidence" in the bisect. At the moment we use how distinct the
       # values are before and after the last broken revision, and how noisy the
       # overall graph is.
       confidence = CalculateConfidence(working_means, broken_means)
 
@@ skipping 514 matching lines @@
       # The perf dashboard scrapes the "results" step in order to comment on
       # bugs. If you change this, please update the perf dashboard as well.
       bisect_utils.OutputAnnotationStepStart('Results')
     print 'Error: %s' % e.message
     if opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
   return 1
 
 if __name__ == '__main__':
   sys.exit(main())