-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core: ensure log-normal score is always in correct range #13392
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,12 @@ | |
*/ | ||
'use strict'; | ||
|
||
// The exact double values for the max and min scores possible in each range. | ||
const MIN_PASSING_SCORE = 0.90000000000000002220446049250313080847263336181640625; | ||
const MAX_AVERAGE_SCORE = 0.899999999999999911182158029987476766109466552734375; | ||
const MIN_AVERAGE_SCORE = 0.5; | ||
const MAX_FAILING_SCORE = 0.499999999999999944488848768742172978818416595458984375; | ||
|
||
/** | ||
* Approximates the Gauss error function, the probability that a random variable | ||
* from the standard normal distribution lies within [-x, x]. Moved from | ||
|
@@ -28,37 +34,6 @@ function erf(x) { | |
return sign * (1 - y * Math.exp(-x * x)); | ||
} | ||
|
||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not clear why this wasn't deleted in #10715. Possibility someone was using it? Hard to let go of our old friend the podr? Searching github the only references I can find that aren't forks of this file are references to forks of this file from four year old forks of Lighthouse, so I think we're good to delete. Farewell podr! You'll live on as an increasingly mystifying reference in our desmos graphs. |
||
* Creates a log-normal distribution à la traceviewer's statistics package. | ||
* Specified by providing the median value, at which the score will be 0.5, | ||
* and the falloff, the initial point of diminishing returns where any | ||
* improvement in value will yield increasingly smaller gains in score. Both | ||
* values should be in the same units (e.g. milliseconds). See | ||
* https://www.desmos.com/calculator/tx1wcjk8ch | ||
* for an interactive view of the relationship between these parameters and | ||
* the typical parameterization (location and shape) of the log-normal | ||
* distribution. | ||
* @param {number} median | ||
* @param {number} falloff | ||
* @return {{computeComplementaryPercentile: function(number): number}} | ||
*/ | ||
function getLogNormalDistribution(median, falloff) { | ||
const location = Math.log(median); | ||
|
||
// The "falloff" value specified the location of the smaller of the positive | ||
// roots of the third derivative of the log-normal CDF. Calculate the shape | ||
// parameter in terms of that value and the median. | ||
const logRatio = Math.log(falloff / median); | ||
const shape = Math.sqrt(1 - 3 * logRatio - Math.sqrt((logRatio - 3) * (logRatio - 3) - 8)) / 2; | ||
|
||
return { | ||
computeComplementaryPercentile(x) { | ||
const standardizedX = (Math.log(x) - location) / (Math.SQRT2 * shape); | ||
return (1 - erf(standardizedX)) / 2; | ||
}, | ||
}; | ||
} | ||
|
||
/** | ||
* Returns the score (1 - percentile) of `value` in a log-normal distribution | ||
* specified by the `median` value, at which the score will be 0.5, and a 10th | ||
|
@@ -76,24 +51,37 @@ function getLogNormalScore({median, p10}, value) { | |
// Required for the log-normal distribution. | ||
if (median <= 0) throw new Error('median must be greater than zero'); | ||
if (p10 <= 0) throw new Error('p10 must be greater than zero'); | ||
// Not required, but if p10 > median, it flips around and becomes the p90 point. | ||
// Not strictly required, but if p10 > median, it flips around and becomes the p90 point. | ||
if (p10 >= median) throw new Error('p10 must be less than the median'); | ||
|
||
// Non-positive values aren't in the distribution, so always 1. | ||
if (value <= 0) return 1; | ||
|
||
// Closest double to `erfc-1(2 * 1/10)`. | ||
// Closest double to `erfc-1(1/5)`. | ||
const INVERSE_ERFC_ONE_FIFTH = 0.9061938024368232; | ||
|
||
// Shape (σ) is `log(p10/median) / (sqrt(2)*erfc^-1(2 * 1/10))` and | ||
// Shape (σ) is `|log(p10/median) / (sqrt(2)*erfc^-1(1/5))|` and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. took me a little while to remember what the |
||
// standardizedX is `1/2 erfc(log(value/median) / (sqrt(2)*σ))`, so simplify a bit. | ||
const xLogRatio = Math.log(value / median); | ||
const p10LogRatio = -Math.log(p10 / median); // negate to keep σ positive. | ||
const xRatio = Math.max(Number.MIN_VALUE, value / median); // value and median are > 0, so is ratio. | ||
const xLogRatio = Math.log(xRatio); | ||
const p10Ratio = Math.max(Number.MIN_VALUE, p10 / median); // p10 and median are > 0, so is ratio. | ||
const p10LogRatio = -Math.log(p10Ratio); // negate to keep σ positive. | ||
Comment on lines
+65
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is unchanged except the extra It's extremely unlikely anyone would ever pick values that would trigger that, but good to have it handled. Interestingly it appears that if a double There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
How did you come to this conclusion? Once you go above any value over MAX_SAFE_INTEGER I find this to be not true.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I suppose I am failing to ensure that each integer has a unique repr. in the double format. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right, strictly |
||
const standardizedX = xLogRatio * INVERSE_ERFC_ONE_FIFTH / p10LogRatio; | ||
const complementaryPercentile = (1 - erf(standardizedX)) / 2; | ||
|
||
// Clamp to [0, 1] to avoid any floating-point out-of-bounds issues. | ||
return Math.min(1, Math.max(0, complementaryPercentile)); | ||
// Clamp to avoid floating-point out-of-bounds issues and keep score in expected range. | ||
let score; | ||
if (value <= p10) { | ||
// Passing. Clamp to [0.9, 1]. | ||
score = Math.max(MIN_PASSING_SCORE, Math.min(1, complementaryPercentile)); | ||
} else if (value <= median) { | ||
// Average. Clamp to [0.5, 0.9). | ||
score = Math.max(MIN_AVERAGE_SCORE, Math.min(MAX_AVERAGE_SCORE, complementaryPercentile)); | ||
} else { | ||
// Failing. Clamp to [0, 0.5). | ||
score = Math.max(0, Math.min(MAX_FAILING_SCORE, complementaryPercentile)); | ||
} | ||
return score; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
|
||
/** | ||
|
@@ -112,6 +100,5 @@ function linearInterpolation(x0, y0, x1, y1, x) { | |
|
||
module.exports = { | ||
linearInterpolation, | ||
getLogNormalDistribution, | ||
getLogNormalScore, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,28 +10,6 @@ | |
const statistics = require('../../lib/statistics.js'); | ||
|
||
describe('statistics', () => { | ||
describe('#getLogNormalDistribution', () => { | ||
it('creates a log normal distribution', () => { | ||
// This curve plotted with the below percentile assertions | ||
// https://www.desmos.com/calculator/vjk2rwd17y | ||
|
||
const median = 5000; | ||
const pODM = 3500; | ||
const dist = statistics.getLogNormalDistribution(median, pODM); | ||
|
||
expect(dist.computeComplementaryPercentile(2000)).toBeCloseTo(1.00); | ||
expect(dist.computeComplementaryPercentile(3000)).toBeCloseTo(0.98); | ||
expect(dist.computeComplementaryPercentile(3500)).toBeCloseTo(0.92); | ||
expect(dist.computeComplementaryPercentile(4000)).toBeCloseTo(0.81); | ||
expect(dist.computeComplementaryPercentile(5000)).toBeCloseTo(0.50); | ||
expect(dist.computeComplementaryPercentile(6000)).toBeCloseTo(0.24); | ||
expect(dist.computeComplementaryPercentile(7000)).toBeCloseTo(0.09); | ||
expect(dist.computeComplementaryPercentile(8000)).toBeCloseTo(0.03); | ||
expect(dist.computeComplementaryPercentile(9000)).toBeCloseTo(0.01); | ||
expect(dist.computeComplementaryPercentile(10000)).toBeCloseTo(0.00); | ||
}); | ||
}); | ||
|
||
describe('#getLogNormalScore', () => { | ||
it('creates a log normal distribution', () => { | ||
// This curve plotted with the below parameters. | ||
|
@@ -44,12 +22,13 @@ describe('statistics', () => { | |
|
||
// Be stricter with the control point requirements. | ||
expect(getLogNormalScore(params, 7300)).toEqual(0.5); | ||
expect(getLogNormalScore(params, 3785)).toBeCloseTo(0.9, 6); | ||
expect(getLogNormalScore(params, 3785)).toEqual(0.9); | ||
|
||
expect(getLogNormalScore(params, 0)).toEqual(1); | ||
expect(getLogNormalScore(params, 1000)).toBeCloseTo(1.00); | ||
expect(getLogNormalScore(params, 2500)).toBeCloseTo(0.98); | ||
expect(getLogNormalScore(params, 5000)).toBeCloseTo(0.77); | ||
expect(getLogNormalScore(params, 7300)).toEqual(0.5); | ||
expect(getLogNormalScore(params, 7500)).toBeCloseTo(0.48); | ||
expect(getLogNormalScore(params, 10000)).toBeCloseTo(0.27); | ||
expect(getLogNormalScore(params, 30000)).toBeCloseTo(0.00); | ||
|
@@ -93,6 +72,73 @@ describe('statistics', () => { | |
statistics.getLogNormalScore({median: 500, p10: 1000}, 50); | ||
}).toThrow('p10 must be less than the median'); | ||
}); | ||
|
||
describe('score is in correct pass/average/fail range', () => { | ||
/** | ||
* Returns the next larger representable double value. | ||
* @type {number} value | ||
*/ | ||
function plusOneUlp(value) { | ||
const f64 = new Float64Array([value]); | ||
const big64 = new BigInt64Array(f64.buffer); | ||
big64[0] += 1n; | ||
return f64[0]; | ||
} | ||
|
||
/** | ||
* Returns the next smaller representable double value. | ||
* @type {number} value | ||
*/ | ||
function minusOneUlp(value) { | ||
if (value === 0) throw new Error(`yeah, can't do that`); | ||
const f64 = new Float64Array([value]); | ||
const big64 = new BigInt64Array(f64.buffer); | ||
big64[0] -= 1n; | ||
return f64[0]; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hello 911 i'd like to report a bit hacker |
||
|
||
const {getLogNormalScore} = statistics; | ||
const controlPoints = [ | ||
{p10: 200, median: 600}, | ||
{p10: 3387, median: 5800}, | ||
{p10: 0.1, median: 0.25}, | ||
{p10: 28 * 1024, median: 128 * 1024}, | ||
{p10: Number.MIN_VALUE, median: plusOneUlp(Number.MIN_VALUE)}, | ||
{p10: Number.MIN_VALUE, median: 21.239999999999977}, | ||
{p10: 99.56000000000073, median: 99.56000000000074}, | ||
{p10: minusOneUlp(Number.MAX_VALUE), median: Number.MAX_VALUE}, | ||
{p10: Number.MIN_VALUE, median: Number.MAX_VALUE}, | ||
]; | ||
|
||
for (const {p10, median} of controlPoints) { | ||
it(`is on the right side of the thresholds for {p10: ${p10}, median: ${median}}`, () => { | ||
const params = {p10, median}; | ||
|
||
// Max 1 at 0, everything else must be ≤ 1. | ||
expect(getLogNormalScore(params, 0)).toEqual(1); | ||
expect(getLogNormalScore(params, plusOneUlp(0))).toBeLessThanOrEqual(1); | ||
|
||
// Just better than passing threshold. | ||
expect(getLogNormalScore(params, minusOneUlp(p10))).toBeGreaterThanOrEqual(0.9); | ||
// At passing threshold. | ||
expect(getLogNormalScore(params, p10)).toEqual(0.9); | ||
// Just worse than passing threshold. | ||
expect(getLogNormalScore(params, plusOneUlp(p10))).toBeLessThan(0.9); | ||
|
||
// Just better than average threshold. | ||
expect(getLogNormalScore(params, minusOneUlp(median))).toBeGreaterThanOrEqual(0.5); | ||
// At average threshold. | ||
expect(getLogNormalScore(params, median)).toEqual(0.5); | ||
// Just worse than passing threshold. | ||
expect(getLogNormalScore(params, plusOneUlp(median))).toBeLessThan(0.5); | ||
|
||
// Some curves never quite reach 0, so just assert some extreme values aren't negative. | ||
expect(getLogNormalScore(params, 1_000_000_000)).toBeGreaterThanOrEqual(0); | ||
expect(getLogNormalScore(params, Number.MAX_SAFE_INTEGER)).toBeGreaterThanOrEqual(0); | ||
expect(getLogNormalScore(params, Number.MAX_VALUE)).toBeGreaterThanOrEqual(0); | ||
}); | ||
} | ||
}); | ||
}); | ||
|
||
describe('#linearInterpolation', () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed for
BigInt64Array
(available since Node 10.8) in a test. If anyone doesn't feel good updating this for the whole project, I can also just addBigInt64Array
as an eslint global to just the test file