Output numbers of run-perf-tests should be rounded in accordance with sample standard deviations

• Component: Tools / Tests
• Status: REOPENED ---

[Kentaro Hara]

Currently the output of performance tests looks like this:

  DESCRIPTION: This benchmark covers DOM attributes that return small integers.
  RESULT Bindings: scroll-top= 191.379586433 runs/s
  median= 191.731644624 runs/s, stdev= 2.56134259787 runs/s, min= 180.412371134 runs/s, max= 192.771084337 runs/s

"191.731644624 runs/s" is too verbose. It should be "191.73 runs/s". Whichever units we use (runs/s or ms), rounding off the output to the second decimal place would be enough and preferable.

[Kentaro Hara]

Created attachment 149726 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 149726 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149726&action=review

> Tools/ChangeLog:16
> +        "191.731644624 runs/s" is too verbose. It should be "191.73 runs/s".
> +        Whichever unit we use (runs/s or ms), rounding off the output to the second
> +        decimal place would be enough and preferable.

No, the whole point of printing out all decimals is so that you can apply arithmetic operations on those numbers and still preserve enough significant figures.
e.g. there are tests that only have 0.7 runs/s for example. For those tests, printing 2 decimals points would mean that we'd get only 2 significant digits. That's not acceptable.
In order to figure out the right significant figures, we need to first figure out the measurement errors.
From measurement errors, we can find out the sufficient significant figures for standard deviations, from which we can find out the right significant figures for the mean, median, etc...

Just picking 2 decimal point arbitrarily isn't an acceptable solution here.

[Ryosuke Niwa]

+WildFox since he would surely know all about significant figures for he is a physicist :)

[Kentaro Hara]

(In reply to comment #2)
> No, the whole point of printing out all decimals is so that you can apply arithmetic operations on those numbers and still preserve enough significant figures.

Are the numbers used for arithmetic calculations? I've thought that _log.info() lines are just for human readability, and the arithmetic calculations (e.g. drawing perf-bot graphs etc) are done by using 'results', which contains all decimals.

[Ryosuke Niwa]

(In reply to comment #4)
> (In reply to comment #2)
> > No, the whole point of printing out all decimals is so that you can apply arithmetic operations on those numbers and still preserve enough significant figures.
> 
> Are the numbers used for arithmetic calculations? I've thought that _log.info() lines are just for human readability, and the arithmetic calculations (e.g. drawing perf-bot graphs etc) are done by using 'results', which contains all decimals.

Yes, I (humans) compute average between different runs all the time.

[Kentaro Hara]

OK, marking as WONTFIX.

[Ryosuke Niwa]

Let's do this properly.

[Ryosuke Niwa]

Created attachment 191049 [details]
Fixes the bug

[Eric Seidel (no email)]

Comment on attachment 191049 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=191049&action=review

> Tools/ChangeLog:12
> +        We show one more digit than the most significant digit in the sample standard deviation since it's
> +        helpful when doing back-of-the-envelop calculations using these numbers.

I have no stats background... but it seems like digits are either significant or not, right?  If you're including non-significant digits in these calculations, isn't that just noise?

> Tools/Scripts/webkitpy/performance_tests/perftest.py:159
> +            return round(x, -highest_decimal_place(x) + sig_fig - 1)

Confused by the -1?

> Tools/Scripts/webkitpy/performance_tests/perftestsrunner_integrationtest.py:91
> +median= 1488 ms, stdev= 1.02 %, min= 1471 ms, max= 1510 ms

Why the change to percent?

[Ryosuke Niwa]

Comment on attachment 191049 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=191049&action=review

>> Tools/ChangeLog:12
>> +        helpful when doing back-of-the-envelop calculations using these numbers.
> 
> I have no stats background... but it seems like digits are either significant or not, right?  If you're including non-significant digits in these calculations, isn't that just noise?

"most significant digit" as in http://planetmath.org/MostSignificantDigit.html
i.e. left most digit of a number when it's shown in the positional notation: http://en.wikipedia.org/wiki/Positional_notation

Maybe there's a better word for it?

>> Tools/Scripts/webkitpy/performance_tests/perftest.py:159
>> +            return round(x, -highest_decimal_place(x) + sig_fig - 1)
> 
> Confused by the -1?

Say we have 1.2345, then to round it at 3rd sig. fig. we do: round(1.2345, 2), not round(1.2345, 3)

[Ryosuke Niwa]

Comment on attachment 191049 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=191049&action=review

>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner_integrationtest.py:91
>> +median= 1488 ms, stdev= 1.02 %, min= 1471 ms, max= 1510 ms
> 
> Why the change to percent?

I find that to be more useful maybe maybe not?

[Eric Seidel (no email)]

The patch looks reasonable, but you should really ask someone who's taken a stats class to review. :)

[Zoltan Horvath]

(In reply to comment #11)
> (From update of attachment 191049 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=191049&action=review
> 
> >> Tools/Scripts/webkitpy/performance_tests/perftestsrunner_integrationtest.py:91
> >> +median= 1488 ms, stdev= 1.02 %, min= 1471 ms, max= 1510 ms
> > 
> > Why the change to percent?
> 
> I find that to be more useful maybe maybe not?

(In reply to comment #12)
> The patch looks reasonable, but you should really ask someone who's taken a stats class to review. :)


I prefer using the percentage, it's more straightforward.

[Ryosuke Niwa]

Comment on attachment 191049 [details]
Fixes the bug

This patch is too old.