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