<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">LGTM.<div><br><div style=""><div>On May 8, 2014, at 1:39 PM, Yi Kong <<a href="mailto:Yi.Kong@arm.com">Yi.Kong@arm.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">I've updated the patch. Sorry for all the code style problems as I'm<br>pretty new to Python.;)<br><br>On Thu, 2014-05-08 at 19:59 +0100, Chris Matthews wrote:<br><blockquote type="cite">@@ -56,6 +58,9 @@ class ComparisonResult:<br><br> def get_value_status(self, confidence_interval=2.576,<br> value_precision=0.0001, ignore_small=True):<br>+ '''<br>+ Raises ImportError if SciPy is not installed and sample size<br>is too large<br>+ '''<br><br>Can you use “”” got the get_value_status doc comment, and also add a<br>period to the end of the sentence.<br><br><br><br><br>+++ b/lnt/server/ui/views.py<br>@@ -191,6 +191,13 @@ class V4RequestInfo(object):<br> 'median' :<br>lnt.util.stats.median }.get(<br> aggregation_fn_name, min)<br><br>+ # Get the MW confidence level<br>+ try:<br>+ confidence_lv =<br>float(request.args.get('MW_confidence_lv'))<br>+ except:<br>+ confidence_lv = .05<br>+ self.confidence_lv = confidence_lv<br>+<br><br>Period at end of comment.<br><br><br>Is there a specific exception you are trying to catch there? I you<br>are expecting a value error? Perhaps make the explicit.<br><br><br>+ try:<br>+ info = V4RequestInfo(id)<br>+ except ImportError:<br>+ return render_template("error.html",<br>+ message="SciPy is not installed on server and sample size<br>is too large")<br>+<br><br><br><br><br>I really like this. It means that the only place we need Scipy is on<br>the servers, not on all the clients.<br><br><br>Period on end of message.<br><br><br>diff --git a/lnt/util/stats.py b/lnt/util/stats.py<br>index a01ad7e..cc23fb2 100644<br>--- a/lnt/util/stats.py<br>+++ b/lnt/util/stats.py<br>@@ -19,3 +19,99 @@ def standard_deviation(l):<br> means_sqrd = sum([(v - m)**2 for v in l]) / len(l)<br> rms = math.sqrt(means_sqrd)<br> return rms<br>+<br><br><br>“”” doc comments, and full sentences for all these.<br><br><br><br><br>+def mannwhitneyu(a, b, sigLevel = .05):<br>+ '''<br>+ Determine if sample a and b are the same at given significance<br>level<br>+ Raises ImportError if SciPy is not installed on server and sample<br>size is<br>+ too large<br>+ '''<br>+ if len(a) <= 20 and len(b) <= 20:<br>+ return mannwhitneyu_small(a, b, sigLevel)<br>+ else:<br>+ try:<br>+ from scipy.stats import mannwhitneyu as<br>mannwhitneyu_large<br>+ return mannwhitneyu_large(a, b, False) >= sigLevel<br>+ except ValueError:<br>+ return True<br>+<br>+def mannwhitneyu_small(a, b, sigLevel):<br>+ '''<br>+ Determine if sample a and b are the same<br>+ Problem size must be less than 20<br>+ '''<br><br><br>Could you have a message on these.<br><br><br>+ assert len(a) <= 20<br>+ assert len(b) <= 20<br>+<br>+<br>+ if not sigLevel in tables:<br>+ raise ValueError("Do not have according significance table")<br><br><br>Period.<br><br><br>+<br>+ flip = len(a) > len(b)<br>+ x = a if not flip else b<br>+ y = b if not flip else a<br>+<br>+ Ux = 0.<br>+ for xe in x:<br>+ for ye in y:<br>+ if xe < ye:<br>+ Ux += 1<br>+ elif xe == ye:<br>+ Ux += .5<br>+ Uy = len(a) * len(b) - Ux<br>+ Ua = Ux if not flip else Uy<br>+ Ub = Uy if not flip else Ux<br>+<br>+ U = abs(Ua - Ub)<br>+ same = U <= tables[sigLevel][len(a) - 1][len(b) - 1]<br>+ return same<br><br><br>Maybe put a description of how this is supposed to work, or a link in<br>the comments?<br><br><br>+# Table for .05 significance level<br><br><br>caps for constant name.<br><br><br>+table_0_05 = [<br>+ [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],<br>+ [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 2, 2, 2, 2],<br>+ [0, 0, 0, 0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8],<br>+ [0, 0, 0, 0, 1, 2, 3, 4, 4, 5, 6, 7, 8, 9, 10, 11, 11, 12,<br>13, 13],<br>+ [0, 0, 0, 1, 2, 3, 5, 6, 7, 8, 9, 11, 12, 13, 14, 15, 17, 18,<br>19, 20],<br>+ [0, 0, 1, 2, 3, 5, 6, 8, 10, 11, 13, 14, 16, 17, 19, 21, 22,<br>24, 25, 27],<br>+ [0, 0, 1, 3, 5, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28,<br>30, 32, 34],<br>+ [0, 0, 2, 4, 6, 8, 10, 13, 15, 17, 19, 22, 24, 26, 29, 31,<br>34, 36, 38, 41],<br>+ [0, 0, 2, 4, 7, 10, 12, 15, 17, 20, 23, 26, 28, 31, 34, 37,<br>39, 42, 45, 48],<br>+ [0, 0, 3, 5, 8, 11, 14, 17, 20, 23, 26, 29, 33, 36, 39, 42,<br>45, 48, 52, 55],<br>+ [0, 0, 3, 6, 9, 13, 16, 19, 23, 26, 30, 33, 37, 40, 44, 47,<br>51, 55, 58, 62],<br>+ [0, 1, 4, 7, 11, 14, 18, 22, 26, 29, 33, 37, 41, 45, 49, 53,<br>57, 61, 65, 69],<br>+ [0, 1, 4, 8, 12, 16, 20, 24, 28, 33, 37, 41, 45, 50, 54, 59,<br>63, 67, 72, 76],<br>+ [0, 1, 5, 9, 13, 17, 22, 26, 31, 36, 40, 45, 50, 55, 59, 64,<br>67, 74, 78, 83],<br>+ [0, 1, 5, 10, 14, 19, 24, 29, 34, 39, 44, 49, 54, 59, 64, 70,<br>75, 80, 85, 90],<br>+ [0, 1, 6, 11, 15, 21, 26, 31, 37, 42, 47, 53, 59, 64, 70, 75,<br>81, 86, 92, 98],<br>+ [0, 2, 6, 11, 17, 22, 28, 34, 39, 45, 51, 57, 63, 67, 75, 81,<br>87, 93, 99, 105],<br>+ [0, 2, 7, 12, 18, 24, 30, 36, 42, 48, 55, 61, 67, 74, 80, 86,<br>93, 99, 106, 112],<br>+ [0, 2, 7, 13, 19, 25, 32, 38, 45, 52, 58, 65, 72, 78, 85, 92,<br>99, 106, 113, 119],<br>+ [0, 2, 8, 13, 20, 27, 34, 41, 48, 55, 62, 69, 76, 83, 90, 98,<br>105, 112, 119, 127]<br>+ ]<br>+<br><br><br>Same.<br><br><br>+# Table for .01 significance level<br>+table_0_01 = [<br>+ [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],<br>+ [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],<br>+ [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 2, 2, 2, 2, 3, 3],<br>+ [0, 0, 0, 0, 0, 0, 0, 1, 1, 2, 2, 3, 3, 4, 5, 5, 6, 6, 7, 8],<br>+ [0, 0, 0, 0, 0, 1, 1, 2, 3, 4, 5, 6, 7, 7, 8, 9, 10, 11, 12,<br>13],<br>+ [0, 0, 0, 0, 1, 2, 3, 4, 5, 6, 7, 9, 10, 11, 12, 13, 15, 16,<br>17, 18],<br>+ [0, 0, 0, 0, 1, 3, 4, 6, 7, 9, 10, 12, 13, 15, 16, 18, 19,<br>21, 22, 24],<br>+ [0, 0, 0, 1, 2, 4, 6, 7, 9, 11, 13, 15, 17, 18, 20, 22, 24,<br>26, 28, 30],<br>+ [0, 0, 0, 1, 3, 5, 7, 9, 11, 13, 16, 18, 20, 22, 24, 27, 29,<br>31, 33, 36],<br>+ [0, 0, 0, 2, 4, 6, 9, 11, 13, 16, 18, 21, 24, 26, 29, 31, 34,<br>37, 39, 42],<br>+ [0, 0, 0, 2, 5, 7, 10, 13, 16, 18, 21, 24, 27, 30, 33, 36,<br>39, 42, 45, 46],<br>+ [0, 0, 1, 3, 6, 9, 12, 15, 18, 21, 24, 27, 31, 34, 37, 41,<br>44, 47, 51, 54],<br>+ [0, 0, 1, 3, 7, 10, 13, 17, 20, 24, 27, 31, 34, 38, 42, 45,<br>49, 53, 56, 60],<br>+ [0, 0, 1, 4, 7, 11, 15, 18, 22, 26, 30, 34, 38, 42, 46, 50,<br>54, 58, 63, 67],<br>+ [0, 0, 2, 5, 8, 12, 16, 20, 24, 29, 33, 37, 42, 46, 51, 55,<br>60, 64, 69, 73],<br>+ [0, 0, 2, 5, 9, 13, 18, 22, 27, 31, 36, 41, 45, 50, 55, 60,<br>65, 70, 74, 79],<br>+ [0, 0, 2, 6, 10, 15, 19, 24, 29, 34, 39, 44, 49, 54, 60, 65,<br>70, 75, 81, 86],<br>+ [0, 0, 2, 6, 11, 16, 21, 26, 31, 37, 42, 47, 53, 58, 64, 70,<br>75, 81, 87, 92],<br>+ [0, 0, 3, 7, 12, 17, 22, 28, 33, 39, 45, 51, 56, 63, 69, 74,<br>81, 87, 93, 99],<br>+ [0, 0, 3, 8, 13, 18, 24, 30, 36, 42, 46, 54, 60, 67, 73, 79,<br>86, 92, 99, 105]<br>+ ]<br>+<br>+tables = {.05: table_0_05, .01: table_0_01}<br><br><br><br>On May 8, 2014, at 2:01 AM, Yi Kong <<a href="mailto:Yi.Kong@arm.com">Yi.Kong@arm.com</a>> wrote:<br><br><blockquote type="cite">Updated again. If SciPy is installed, it will use normal<br>approximation<br>to compute MWU. Otherwise it will show an error page in large<br>problem<br>sizes.<br><br>On Wed, 2014-05-07 at 17:16 +0100, Hal Finkel wrote:<br><blockquote type="cite">----- Original Message -----<br><blockquote type="cite">From: "Yi Kong" <<a href="mailto:Yi.Kong@arm.com">Yi.Kong@arm.com</a>><br>To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>Cc: "LLVM Commits" <<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>>, "Renato Golin"<br><<a href="mailto:renato.golin@linaro.org">renato.golin@linaro.org</a>>, "Anton Korobeynikov"<br><<a href="mailto:anton@korobeynikov.info">anton@korobeynikov.info</a>>, "Chris Matthews"<br><<a href="mailto:chris.matthews@apple.com">chris.matthews@apple.com</a>><br>Sent: Wednesday, May 7, 2014 11:06:36 AM<br>Subject: Re: [LNT] r207898 - Use Mann-Whitney U test to identify<br>changes<br><br>Updated. If the sample size if greater than 20, Mann-Whitney U<br>test<br>won't be performed.<br></blockquote><br>Sorry, I was not clear...<br><br>+ # Use Mann-Whitney U test to test null hypothesis that<br>result is<br><br>+ # unchanged.<br><br>+ if len(self.samples) >= 4 and len(self.samples) <= 20 and<br>\<br><br>+ len(self.prev_samples) >= 4 and len(self.prev_samples) <=<br>20:<br><br>+ same = stats.mannwhitneyu(self.samples,<br>self.prev_samples, self.confidence_lv)<br><br>+ if same:<br><br>+ return UNCHANGED_PASS<br><br>This is going to be very confusing; in theory, this means that<br>increasing the number of samples will give you better results<br>until you hit 20, and then suddenly you'll get meaningless<br>answers. I had meant that it should produce an *error* if you even<br>try to run such a configuration.<br><br>-Hal<br><br><blockquote type="cite"><br>On Wed, 2014-05-07 at 16:12 +0100, Hal Finkel wrote:<br><blockquote type="cite">----- Original Message -----<br><blockquote type="cite">From: "Yi Kong" <<a href="mailto:Yi.Kong@arm.com">Yi.Kong@arm.com</a>><br>To: "Anton Korobeynikov" <<a href="mailto:anton@korobeynikov.info">anton@korobeynikov.info</a>>, "Chris<br>Matthews" <<a href="mailto:chris.matthews@apple.com">chris.matthews@apple.com</a>><br>Cc: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>>, "LLVM Commits"<br><<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>>, "Renato Golin"<br><<a href="mailto:renato.golin@linaro.org">renato.golin@linaro.org</a>><br>Sent: Wednesday, May 7, 2014 10:02:11 AM<br>Subject: Re: [LNT] r207898 - Use Mann-Whitney U test to<br>identify<br>changes<br><br>I've updated the patch to perform Mann-Whitney U test using<br>significance<br>table. We still need SciPy when sample size is large(> 20).<br></blockquote><br>Why do you still require SciPy? That is a large package to<br>pull in<br>for one rarely-used function. Are there other parts of SciPy<br>that<br>you anticipate us using in the future? If not, then there are<br>two<br>options (which I think are both reasonable):<br><br>1. Limit the number of repeat samples taken to 20 (running the<br>test suite more than 20 times per revision seems unlikely in<br>practice).<br>2. Implement the normal approximation to the U-value<br>calculation<br>in the code. From the description on the wikipedia page, the<br>algorithm seems pretty simple.<br><br>-Hal<br><br><blockquote type="cite"><br>On Wed, 2014-05-07 at 09:12 +0100, Anton Korobeynikov wrote:<br><blockquote type="cite"><blockquote type="cite">If we are using significance table, we can no longer<br>calculate p<br>values,<br>right? Is there any algorithm to calculate p value for<br>all<br>sample<br>sizes?<br></blockquote>We would just need to fix the threshold (say, 0.05 or 0.1<br>or<br>0.01).<br>Also, we can have like 2 or 3 tables for various<br>thresholds.<br>This<br>should be enough for all the practical purposes.<br><br></blockquote><br><br><br></blockquote><br></blockquote><br></blockquote><br></blockquote><br><br>-- IMPORTANT NOTICE: The contents of this email and any attachments<br>are confidential and may also be privileged. If you are not the<br>intended recipient, please notify the sender immediately and do not<br>disclose the contents to any other person, use it for any purpose,<br>or store or copy the information in any medium. Thank you.<br><br>ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,<br>Registered in England & Wales, Company No: 2557590<br>ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1<br>9NJ, Registered in England & Wales, Company No:<br>2548782<correct_mw_dyn_import.diff><br></blockquote><br><br></blockquote><span><0001-Use-Mann-Whitney-U-test-to-identify-changes-2.patch></span></div></blockquote></div><br></div></body></html>