Skip to content

MAINT: Update quantile default lerp method#20331

Merged
seberg merged 2 commits intonumpy:mainfrom
abel-bzz:fix/quantile-default-lerp-method
Nov 9, 2021
Merged

MAINT: Update quantile default lerp method#20331
seberg merged 2 commits intonumpy:mainfrom
abel-bzz:fix/quantile-default-lerp-method

Conversation

@abel-bzz
Copy link
Copy Markdown
Contributor

@abel-bzz abel-bzz commented Nov 9, 2021

For method 7 of H&F, using (n - 1) * quantiles gives a more accurate result than
_compute_virtual_index(n, quantiles, 1, 1).

It should fix: pandas-dev/pandas#44343

For method 7 of H&F, using `(n - 1) * quantiles`
instead of the usual method gives a more accurate
result.
@seberg
Copy link
Copy Markdown
Member

seberg commented Nov 9, 2021

Should/can we add a test for this thing by checking that the quantile of data and the quantile of the data repeated a few times is identical?
Just wondering, also happy to just put it in, pandas tests "cover" it anyway.

@seberg seberg added this to the 1.22.0 release milestone Nov 9, 2021
@abel-bzz
Copy link
Copy Markdown
Contributor Author

abel-bzz commented Nov 9, 2021

I was wondering how to test it.
Would that be enough ?

a = _compute_virtual_index(n, q, 1, 1)
b = (n-1)* q

np.testing.assert_almost_equal(a, b, )

@seberg
Copy link
Copy Markdown
Member

seberg commented Nov 9, 2021

Yeah, you are right. I didn't realize that the reason for the difference was that pandas probably implements its own quantile version there and ULPs just add up slightly differently. Lets see if this helps pandas. If not, I expect pandas should relax their test.

The only test I could think of is asserting a specific result with max-ULPs...

@seberg seberg force-pushed the fix/quantile-default-lerp-method branch from 0176a19 to 53e3df3 Compare November 9, 2021 20:42
@seberg
Copy link
Copy Markdown
Member

seberg commented Nov 9, 2021

Thanks! I dumbed the test down and just hardcoded the expected value, will merge once tests pass. Hopefully the 1ULP won't fail for some reason, but I will expect that IEEE compliant doubles will always lead to this...

@seberg seberg changed the title MTH: Update quantile default lerp method MAINT: Update quantile default lerp method Nov 9, 2021
@seberg
Copy link
Copy Markdown
Member

seberg commented Nov 9, 2021

Thanks @bzah (the MacOS job timed out, going to ignore that, maybe bumping the mac os version will help with that job being so slow recently).

@seberg seberg merged commit a0d86e2 into numpy:main Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: New Numpydev version broke ci

2 participants