<div dir="ltr">Hi Chris,<br><br>I just updated our LNT instance and this commit causes the following internal server error:<br><div><br></div><div>... snip ...</div><div><div>  File "/home/llvm-test/sandbox/lnt/lnt/server/ui/templates/v4_run.html", line 334, in block "body"</div><div>    {% set cr = request_info.sri.get_geomean_comparison_result(</div><div>  File "/home/llvm-test/sandbox/lnt/lnt/server/reporting/analysis.py", line 286, in get_geomean_comparison_result</div><div>    bigger_is_better=field.bigger_is_better)</div><div>  File "/home/llvm-test/sandbox/lnt/lnt/server/reporting/analysis.py", line 60, in __init__</div><div>    self.stddev = stats.standard_deviation(samples)</div><div>  File "/home/llvm-test/sandbox/lnt/lnt/util/stats.py", line 36, in standard_deviation</div><div>    m = mean(l)</div><div>  File "/home/llvm-test/sandbox/lnt/lnt/util/stats.py", line 15, in mean</div><div>    return sum(l)/len(l)</div><div>TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'</div></div><div><br></div><div>Could you please take a look?</div><div><br></div><div>Cheers,</div><div><br></div><div>James</div><div><br></div><div>[P.S: if you reply to <a href="mailto:james.molloy@arm.com">james.molloy@arm.com</a> perhaps I might get your reply - I still lay the blame at gmail's forwarding eating your messages]</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, 19 May 2015 at 03:11 Chris Matthews <<a href="mailto:cmatthews5@apple.com">cmatthews5@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: cmatthews<br>
Date: Mon May 18 20:59:20 2015<br>
New Revision: 237658<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D237658-26view-3Drev&d=AwMGaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=tOwN081Lt-Jz4TOyqawc7NfTNmiZHeWSjyvCCIyugCE&s=x63f7KV2RTxHIzS8IndcX1JyI7zgDc8163KQnG-9YlY&e=" target="_blank">http://llvm.org/viewvc/llvm-project?rev=237658&view=rev</a><br>
Log:<br>
Refactor ComparisonResult to be more self contained<br>
<br>
Modified:<br>
    lnt/trunk/lnt/server/reporting/analysis.py<br>
    lnt/trunk/lnt/server/ui/templates/v4_run.html<br>
    lnt/trunk/tests/server/reporting/analysis.py<br>
<br>
Modified: lnt/trunk/lnt/server/reporting/analysis.py<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lnt_trunk_lnt_server_reporting_analysis.py-3Frev-3D237658-26r1-3D237657-26r2-3D237658-26view-3Ddiff&d=AwMGaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=tOwN081Lt-Jz4TOyqawc7NfTNmiZHeWSjyvCCIyugCE&s=6N1OogQVs75Hj_cyGGHkmpIo73TxJYLwd6d67GjBcT4&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/server/reporting/analysis.py?rev=237658&r1=237657&r2=237658&view=diff</a><br>
==============================================================================<br>
--- lnt/trunk/lnt/server/reporting/analysis.py (original)<br>
+++ lnt/trunk/lnt/server/reporting/analysis.py Mon May 18 20:59:20 2015<br>
@@ -31,43 +31,67 @@ def calc_geomean(run_values):<br>
 class ComparisonResult:<br>
     """A ComparisonResult is ultimatly responsible for determining if a test<br>
     improves, regresses or does not change, given some new and old data."""<br>
-<br>
-    def __init__(self,cur_value, prev_value, delta, pct_delta, stddev, MAD,<br>
-                 cur_failed, prev_failed, samples, prev_samples, stddev_mean = None,<br>
-                 confidence_lv = .05, bigger_is_better = False):<br>
-        self.current = cur_value<br>
-        self.previous = prev_value<br>
-        self.delta = delta<br>
-        self.pct_delta = pct_delta<br>
-        self.stddev = stddev<br>
-        self.MAD = MAD<br>
+<br>
+    def __init__(self, aggregation_fn,<br>
+                 cur_failed, prev_failed, samples, prev_samples,<br>
+                 confidence_lv=0.05, bigger_is_better=False):<br>
+        self.aggregation_fn = aggregation_fn<br>
+        if samples:<br>
+            self.current = aggregation_fn(samples)<br>
+        else:<br>
+            self.current = None<br>
+        if prev_samples:<br>
+            self.previous = aggregation_fn(prev_samples)<br>
+        else:<br>
+            self.previous = None<br>
+<br>
+        # Compute the comparison status for the test value.<br>
+        if self.current and self.previous and self.previous != 0:<br>
+            self.delta = self.current - self.previous<br>
+            self.pct_delta = self.delta / self.previous<br>
+        else:<br>
+            self.delta = 0<br>
+            self.pct_delta = 0.0<br>
+<br>
+        # If we have multiple values for this run, use that to estimate the<br>
+        # distribution.<br>
+        if samples and len(samples) > 1:<br>
+            self.stddev = stats.standard_deviation(samples)<br>
+            self.MAD = stats.median_absolute_deviation(samples)<br>
+        else:<br>
+            self.stddev = None<br>
+            self.MAD = None<br>
+<br>
+        self.stddev_mean = None  # Only calculate this if needed.<br>
         self.failed = cur_failed<br>
         self.prev_failed = prev_failed<br>
         self.samples = samples<br>
         self.prev_samples = prev_samples<br>
-        self.stddev_mean = stddev_mean<br>
+<br>
         self.confidence_lv = confidence_lv<br>
         self.bigger_is_better = bigger_is_better<br>
<br>
+    @property<br>
+    def stddev_mean(self):<br>
+        """The mean around stddev for current sampples. Cached after first call.<br>
+        """<br>
+        if not self.stddev_mean:<br>
+            self.stddev_mean = stats.mean(self.samples)<br>
+        return self.stddev_mean<br>
+<br>
     def __repr__(self):<br>
         """Print this ComparisonResult's constructor.<br>
-<br>
+<br>
         Handy for generating test cases for comparisons doing odd things."""<br>
-        frmt = "{}(" + "{}, " * 11 + ")"<br>
-        return frmt.format("ComparisonResult",<br>
-                           self.current,<br>
-                           self.previous,<br>
-                           self.delta,<br>
-                           self.pct_delta,<br>
-                           self.stddev,<br>
-                           self.MAD,<br>
-                           self.failed,<br>
-                           self.prev_failed,<br>
-                           self.samples,<br>
-                           self.prev_samples,<br>
-                           self.stddev_mean,<br>
-                           self.confidence_lv,<br>
-                           self.bigger_is_better)<br>
+        fmt = "{}(" + "{}, " * 7 + ")"<br>
+        return fmt.format(self.__class__.__name__,<br>
+                          self.aggregation_fn.__name__,<br>
+                          self.failed,<br>
+                          self.prev_failed,<br>
+                          self.samples,<br>
+                          self.prev_samples,<br>
+                          self.confidence_lv,<br>
+                          bool(self.bigger_is_better))<br>
<br>
     def is_result_interesting(self):<br>
         """is_result_interesting() -> bool<br>
@@ -237,77 +261,27 @@ class RunInfo(object):<br>
                       if s[field.index] is not None]<br>
         prev_values = [s[field.index] for s in prev_samples<br>
                        if s[field.index] is not None]<br>
-        if run_values:<br>
-            run_value = self.aggregation_fn(run_values)<br>
-        else:<br>
-            run_value = None<br>
-        if prev_values:<br>
-            prev_value = self.aggregation_fn(prev_values)<br>
-        else:<br>
-            prev_value = None<br>
-<br>
-        # If we have multiple values for this run, use that to estimate the<br>
-        # distribution.<br>
-        if run_values and len(run_values) > 1:<br>
-            stddev = stats.standard_deviation(run_values)<br>
-            MAD = stats.median_absolute_deviation(run_values)<br>
-            stddev_mean = stats.mean(run_values)<br>
-        else:<br>
-            stddev = None<br>
-            MAD = None<br>
-            stddev_mean = None<br>
-<br>
-        # If we are missing current or comparison values we are done.<br>
-        if run_value is None or prev_value is None:<br>
-            return ComparisonResult(<br>
-                run_value, prev_value, delta=None,<br>
-                pct_delta = None, stddev = stddev, MAD = MAD,<br>
-                cur_failed = run_failed, prev_failed = prev_failed,<br>
-                samples = run_values, prev_samples = prev_values,<br>
-                confidence_lv = self.confidence_lv,<br>
-                bigger_is_better = field.bigger_is_better)<br>
-<br>
-        # Compute the comparison status for the test value.<br>
-        delta = run_value - prev_value<br>
-        if prev_value != 0:<br>
-            pct_delta = delta / prev_value<br>
-        else:<br>
-            pct_delta = 0.0<br>
-<br>
-        return ComparisonResult(run_value, prev_value, delta,<br>
-                                pct_delta, stddev, MAD,<br>
-                                run_failed, prev_failed, run_values,<br>
-                                prev_values, stddev_mean, self.confidence_lv,<br>
-                                bigger_is_better = field.bigger_is_better)<br>
-<br>
+<br>
+        r = ComparisonResult(self.aggregation_fn,<br>
+                             run_failed, prev_failed, run_values,<br>
+                             prev_values, self.confidence_lv,<br>
+                             bigger_is_better=field.bigger_is_better)<br>
+        print repr(r)<br>
+        return r<br>
<br>
-    def get_geomean_comparison_result(self, run, compare_to, field, tests,<br>
-                                      comparison_window=[]):<br>
+    def get_geomean_comparison_result(self, run, compare_to, field, tests):<br>
         if tests:<br>
             prev_values,run_values = zip(*[(cr.previous,cr.current) for _,_,cr in tests])<br>
         else:<br>
             prev_values,run_values = [], []<br>
<br>
-        run_geomean = calc_geomean(run_values)<br>
-        prev_geomean = calc_geomean(prev_values)<br>
-<br>
-        if run_geomean and prev_geomean:<br>
-            delta = run_geomean - prev_geomean<br>
-            if prev_geomean != 0:<br>
-                pct_delta = delta / prev_geomean<br>
-            else:<br>
-                pct_delta = 0.0<br>
-        else:<br>
-            delta = pct_delta = 0<br>
-<br>
-        return ComparisonResult(run_geomean, prev_geomean, delta,<br>
-                                pct_delta, stddev=None, MAD=None,<br>
-                                cur_failed=run_values and not run_geomean,<br>
-                                prev_failed=prev_values and not prev_geomean,<br>
-                                samples=[run_geomean] if run_geomean else [],<br>
-                                prev_samples=[prev_geomean] if prev_geomean else [],<br>
+        return ComparisonResult(calc_geomean,<br>
+                                cur_failed=bool(run_values),<br>
+                                prev_failed=bool(prev_values),<br>
+                                samples=run_values,<br>
+                                prev_samples=prev_values,<br>
                                 confidence_lv=0,<br>
-                                bigger_is_better = field.bigger_is_better)<br>
+                                bigger_is_better=field.bigger_is_better)<br>
<br>
     def _load_samples_for_runs(self, run_ids):<br>
         # Find the set of new runs to load.<br>
<br>
Modified: lnt/trunk/lnt/server/ui/templates/v4_run.html<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lnt_trunk_lnt_server_ui_templates_v4-5Frun.html-3Frev-3D237658-26r1-3D237657-26r2-3D237658-26view-3Ddiff&d=AwMGaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=tOwN081Lt-Jz4TOyqawc7NfTNmiZHeWSjyvCCIyugCE&s=bxa1O3Lgv6PWPyzsfquubZlDJHyQUTyd8s6mnu02a9M&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/server/ui/templates/v4_run.html?rev=237658&r1=237657&r2=237658&view=diff</a><br>
==============================================================================<br>
--- lnt/trunk/lnt/server/ui/templates/v4_run.html (original)<br>
+++ lnt/trunk/lnt/server/ui/templates/v4_run.html Mon May 18 20:59:20 2015<br>
@@ -332,7 +332,7 @@<br>
         </tbody><br>
         <tfoot><br>
           {% set cr = request_info.sri.get_geomean_comparison_result(<br>
-                      run, compare_to, field, tests, request_info.comparison_window) %}<br>
+                      run, compare_to, field, tests) %}<br>
           <td><input type="checkbox" name="mean" value="{{<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__machine.id&d=AwMGaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=tOwN081Lt-Jz4TOyqawc7NfTNmiZHeWSjyvCCIyugCE&s=3nH6a08H7S9rGyQsRjJhVerVSexFIhOErWUKuT2g3Rk&e=" target="_blank">machine.id</a>}}.{{field.index}}"></td><br>
           <td><a href="{{graph_base}}&mean={{<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__machine.id&d=AwMGaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=tOwN081Lt-Jz4TOyqawc7NfTNmiZHeWSjyvCCIyugCE&s=3nH6a08H7S9rGyQsRjJhVerVSexFIhOErWUKuT2g3Rk&e=" target="_blank">machine.id</a>}}.{{field.index}}">Geometric Mean</a></td><br>
           {{ get_cell_value(cr) }}<br>
<br>
Modified: lnt/trunk/tests/server/reporting/analysis.py<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lnt_trunk_tests_server_reporting_analysis.py-3Frev-3D237658-26r1-3D237657-26r2-3D237658-26view-3Ddiff&d=AwMGaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=tOwN081Lt-Jz4TOyqawc7NfTNmiZHeWSjyvCCIyugCE&s=8F12Bu-wXK5zEmSCxRH5mQaZ4BDU8I_1yKGCAa5hwPc&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lnt/trunk/tests/server/reporting/analysis.py?rev=237658&r1=237657&r2=237658&view=diff</a><br>
==============================================================================<br>
--- lnt/trunk/tests/server/reporting/analysis.py (original)<br>
+++ lnt/trunk/tests/server/reporting/analysis.py Mon May 18 20:59:20 2015<br>
@@ -4,7 +4,7 @@<br>
 import unittest<br>
 import lnt.util.stats as stats<br>
 from lnt.server.reporting.analysis import ComparisonResult, REGRESSED, IMPROVED<br>
-from lnt.server.reporting.analysis import UNCHANGED_PASS<br>
+from lnt.server.reporting.analysis import UNCHANGED_PASS, UNCHANGED_FAIL<br>
<br>
<br>
 class ComparisonResultTest(unittest.TestCase):<br>
@@ -13,15 +13,8 @@ class ComparisonResultTest(unittest.Test<br>
     def test_comp(self):<br>
         """Test a real example."""<br>
         curr_samples = [0.0887, 0.0919, 0.0903]<br>
-        prev = 0.0858<br>
-        cur = min(curr_samples)<br>
-        stddev = stats.standard_deviation(curr_samples)<br>
-        MAD = stats.median_absolute_deviation(curr_samples)<br>
-        stddev_mean = stats.mean(curr_samples)<br>
-        uninteresting = ComparisonResult(cur, prev, cur-prev,<br>
-                                         (cur-prev)/prev, stddev, MAD,<br>
-                                         False, False, curr_samples, [prev],<br>
-                                         stddev_mean)<br>
+        prev = [0.0858]<br>
+        uninteresting = ComparisonResult(min, False, False, curr_samples, prev)<br>
<br>
         self.assertFalse(uninteresting.is_result_interesting())<br>
         self.assertEquals(uninteresting.get_test_status(), UNCHANGED_PASS)<br>
@@ -29,31 +22,36 @@ class ComparisonResultTest(unittest.Test<br>
<br>
     def test_slower(self):<br>
         """Test getting a simple regression."""<br>
-        slower = ComparisonResult(10, 5, 5, 0.5, None, None,<br>
-                                  False, False, [10], [5], None)<br>
+        slower = ComparisonResult(min,<br>
+                                  False, False, [10], [5])<br>
         self.assertEquals(slower.get_value_status(), REGRESSED)<br>
         self.assertTrue(slower.is_result_interesting())<br>
<br>
     def test_faster(self):<br>
         """Test getting a simple improvement."""<br>
<br>
-        faster = ComparisonResult(5, 10, -5, -0.5, None, None,<br>
-                                  False, False, [5], [10], None)<br>
+        faster = ComparisonResult(min,<br>
+                                  False, False, [5], [10])<br>
         self.assertEquals(faster.get_value_status(), IMPROVED)<br>
         self.assertTrue(faster.is_result_interesting())<br>
<br>
     def test_improved_status(self):<br>
         """Test getting a test status improvement."""<br>
-        improved = ComparisonResult(None, None, None, None, None, None,<br>
-                                    False, True, [5], [10], None)<br>
+        improved = ComparisonResult(min,<br>
+                                    False, True, [1], None)<br>
         self.assertEquals(improved.get_test_status(), IMPROVED)<br>
<br>
     def test_regressed_status(self):<br>
         """Test getting a test status improvement."""<br>
-        improved = ComparisonResult(None, None, None, None, None, None,<br>
-                                    True, False, [5], [10], None)<br>
+        improved = ComparisonResult(min,<br>
+                                    True, False, None, [10])<br>
         self.assertEquals(improved.get_test_status(), REGRESSED)<br>
<br>
+    def test_keep_on_failing_status(self):<br>
+        """Test getting a repeated fail."""<br>
+        improved = ComparisonResult(min,<br>
+                                    True, True, None, None)<br>
+        self.assertEquals(improved.get_test_status(), UNCHANGED_FAIL)<br>
<br>
 if __name__ == '__main__':<br>
     unittest.main()<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>