<div dir="ltr"><div>Hi Chris,</div><div><br></div><div>This change has some questionable behavior and works pretty poorly when dealing with instances that have reasonable sample counts per run (e.g., 10).</div><div><br></div><div>Given this ComparisonResult:</div><div> ComparisonResult(safe_min, False, False, [0.873164, 0.822148, 0.877115, 0.830048, 0.830452, 0.897407, 0.817558, 0.842067, 0.836447, 0.875904], [0.695299, 0.798883, 0.676503, 0.671114, 0.648465, 0.653936, 0.673405, 0.646322, 0.654594, 0.667564], 0.05, False, )</div><div>where all the previous samples are from the immediately preceding run (not from multiple runs), then this ends up computing:</div><div>--</div><div>cr.current == 0.8176</div><div>cr.previous == 0.6463</div><div>cr.delta == 0.0187</div><div>cr.pct_delta == 2.34%</div><div>--</div><div>which doesn't make any sense to the user who is just seeing those values and wondering where the strange delta value came from.</div><div><br></div><div>If we are going to keep this behavior, then I think that:</div><div>a. The cr.previous value should be updated to be the selected value, so that all the values are at least consistent in the UI.</div><div>b. This behavior should only be enabled when using prior samples across runs.</div><div><br></div><div>I would also prefer this was modeled as a different "aggregation function" that shows in the UI, and maybe make that an per-instance default.</div><div><br></div><div> - Daniel</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 3, 2015 at 2:01 PM, Chris Matthews <span dir="ltr"><<a href="mailto:cmatthews5@apple.com" target="_blank">cmatthews5@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: cmatthews<br>
Date: Wed Jun 3 16:01:23 2015<br>
New Revision: 238965<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=238965&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=238965&view=rev</a><br>
Log:<br>
Improved regression detection with min of diffs<br>
<br>
This change reduces false positives in LNT's regression detection<br>
algorithm by having it use more past data. Instead of calculating the<br>
delta as the difference of min(current)-min(prev), we use the smallest<br>
difference between min(current) and all prevs. When the data has noise<br>
or is multimodal this reduces our chances of flagging a false positive<br>
regression.<br>
<br>
Modified:<br>
lnt/trunk/lnt/server/reporting/analysis.py<br>
lnt/trunk/tests/server/reporting/analysis.py<br>
<br>
Modified: lnt/trunk/lnt/server/reporting/analysis.py<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/server/reporting/analysis.py?rev=238965&r1=238964&r2=238965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/server/reporting/analysis.py?rev=238965&r1=238964&r2=238965&view=diff</a><br>
==============================================================================<br>
--- lnt/trunk/lnt/server/reporting/analysis.py (original)<br>
+++ lnt/trunk/lnt/server/reporting/analysis.py Wed Jun 3 16:01:23 2015<br>
@@ -15,6 +15,21 @@ UNCHANGED_FAIL = 'UNCHANGED_FAIL'<br>
MIN_VALUE_PRECISION = 0.0001<br>
<br>
<br>
+def absmin_diff(current, prevs):<br>
+ """Min of differences between current sample and all previous samples.<br>
+ Given more than one min, use the last one detected which is probably a<br>
+ newer value. Returns (difference, prev used)<br>
+ """<br>
+ diffs = [abs(current-prev) for prev in prevs]<br>
+ smallest_pos = 0<br>
+ smallest = diffs[0]<br>
+ for i, diff in enumerate(diffs):<br>
+ if diff <= smallest:<br>
+ smallest = diff<br>
+ smallest_pos = i<br>
+ return current-prevs[smallest_pos], prevs[smallest_pos]<br>
+<br>
+<br>
def calc_geomean(run_values):<br>
# NOTE Geometric mean applied only to positive values, so fix it by<br>
# adding MIN_VALUE to each value and substract it from the result.<br>
@@ -48,8 +63,8 @@ class ComparisonResult:<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>
+ self.delta, value = absmin_diff(self.current, prev_samples)<br>
+ self.pct_delta = self.delta / value<br>
else:<br>
self.delta = 0<br>
self.pct_delta = 0.0<br>
<br>
Modified: lnt/trunk/tests/server/reporting/analysis.py<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lnt/trunk/tests/server/reporting/analysis.py?rev=238965&r1=238964&r2=238965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lnt/trunk/tests/server/reporting/analysis.py?rev=238965&r1=238964&r2=238965&view=diff</a><br>
==============================================================================<br>
--- lnt/trunk/tests/server/reporting/analysis.py (original)<br>
+++ lnt/trunk/tests/server/reporting/analysis.py Wed Jun 3 16:01:23 2015<br>
@@ -2,8 +2,10 @@<br>
#<br>
# RUN: python %s<br>
import unittest<br>
+<br>
from lnt.server.reporting.analysis import ComparisonResult, REGRESSED, IMPROVED<br>
from lnt.server.reporting.analysis import UNCHANGED_PASS, UNCHANGED_FAIL<br>
+from lnt.server.reporting.analysis import absmin_diff<br>
<br>
FLAT_LINE = [1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0,<br>
1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0]<br>
@@ -309,5 +311,18 @@ class ComparisonResultTest(unittest.Test<br>
# Fixme<br>
# self.assertEquals(slow.get_value_status(), IMPROVED)<br>
<br>
+<br>
+class AbsMinTester(unittest.TestCase):<br>
+<br>
+ def test_absmin(self):<br>
+ """Test finding smallest difference."""<br>
+ self.assertEqual(absmin_diff(1, [2, 2, 3]), (-1, 2))<br>
+ self.assertEqual(absmin_diff(1, [1, 2, 3]), (0, 1))<br>
+ self.assertEqual(absmin_diff(1, [2]), (-1, 2))<br>
+ self.assertEqual(absmin_diff(1.5, [1, 4, 4]), (0.5, 1))<br>
+ self.assertEqual(absmin_diff(5, [1, 2, 1]), (3, 2))<br>
+ self.assertEqual(absmin_diff(1, [2, 0, 3]), (1, 0))<br>
+<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">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>