Skip to content
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

BUG: Fix np.quantile([Fraction(2,1)], 0.5) #24711

Merged
merged 3 commits into from Jan 30, 2024

Conversation

eendebakpt
Copy link
Contributor

@eendebakpt eendebakpt commented Sep 14, 2023

On main:

import numpy as np
from fractions import Fraction
arr = np.array([Fraction(1, 1), Fraction(2, 1), Fraction(3, 1)])
np.quantile(arr, 0) # output: Fraction(1, 1)
np.quantile(arr, Fraction(1, 2)) # output: Fraction(2, 1)
np.quantile(arr, 0.5) # raises an error

In this PR we update the np.quantile to handle the case np.quantile(arr, 0.5) as well.

Also see #24592

@eendebakpt eendebakpt changed the title Draft: BUG: Fix np.quantile([Fraction(2,1)], 0.5) BUG: Fix np.quantile([Fraction(2,1)], 0.5) Sep 14, 2023
@charris
Copy link
Member

charris commented Sep 15, 2023

The debug test failure seems to be an Ubuntu problem, the requested file is missing from the upstream directory. I suspect it is due to a micro upgrade in the Ubuntu version.

@@ -4579,7 +4579,8 @@ def _lerp(a, b, t, out=None):
diff_b_a = subtract(b, a)
# asanyarray is a stop-gap until gh-13105
lerp_interpolation = asanyarray(add(a, diff_b_a * t, out=out))
subtract(b, diff_b_a * (1 - t), out=lerp_interpolation, where=t >= 0.5)
subtract(b, diff_b_a * (1 - t), out=lerp_interpolation, where=t >= 0.5,
casting='unsafe')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may sound weird, but we should probably also add dtype=type(lerp_interpolation.dtype).

If we have unsafe casting, this means that at least to some degree we ignore the precision of the weights of the quantiles for the output precision (promotion). Can you give me a hint in how things are currently? Does the same happen if we use np.longdouble for the quantile level (q)?

(Sorry, but I am a bit surprised how many subtleties there still seem to be along promotion in quantiles :(, and hoping we can clarify things a bit for me and everyone else.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seberg I tested with

import numpy as np
print(np,np.__version__)

x = np.linspace(.1, .7, 5, dtype=np.longdouble)
for q in [0, 1, 0., 1., .5]:
    r = np.quantile(x, q)
    print(f'{q} ({type(q)}) -> {r} ({type(r)})')
    r = np.quantile(x, np.longdouble(q))
    print(f'{q} ({type(q)}) -> {r} ({type(r)})')
    
    
print('----')    
x = np.array([0, 1])
for q in [0, 1, 0., 1., .5]:
    r = np.quantile(x, q)
    print(f'{q} ({type(q)}) -> {r} ({type(r)})')
    r = np.quantile(x, np.longdouble(q))
    print(f'{q} ({type(q)}) -> {r} ({type(r)})')    

Output on 1.26:

<module 'numpy' from '/home/eendebakpt/py310/lib/python3.10/site-packages/numpy/__init__.py'> 1.26.0
0 (<class 'int'>) -> 0.1 (<class 'numpy.longdouble'>)
0 (<class 'int'>) -> 0.1 (<class 'numpy.longdouble'>)
1 (<class 'int'>) -> 0.7 (<class 'numpy.longdouble'>)
1 (<class 'int'>) -> 0.7 (<class 'numpy.longdouble'>)
0.0 (<class 'float'>) -> 0.1 (<class 'numpy.longdouble'>)
0.0 (<class 'float'>) -> 0.1 (<class 'numpy.longdouble'>)
1.0 (<class 'float'>) -> 0.7 (<class 'numpy.longdouble'>)
1.0 (<class 'float'>) -> 0.7 (<class 'numpy.longdouble'>)
0.5 (<class 'float'>) -> 0.4 (<class 'numpy.longdouble'>)
0.5 (<class 'float'>) -> 0.4 (<class 'numpy.longdouble'>)
----
0 (<class 'int'>) -> 0 (<class 'numpy.int64'>)
0 (<class 'int'>) -> 0.0 (<class 'numpy.longdouble'>)
1 (<class 'int'>) -> 1 (<class 'numpy.int64'>)
1 (<class 'int'>) -> 1.0 (<class 'numpy.longdouble'>)
0.0 (<class 'float'>) -> 0.0 (<class 'numpy.float64'>)
0.0 (<class 'float'>) -> 0.0 (<class 'numpy.longdouble'>)
1.0 (<class 'float'>) -> 1.0 (<class 'numpy.float64'>)
1.0 (<class 'float'>) -> 1.0 (<class 'numpy.longdouble'>)
0.5 (<class 'float'>) -> 0.5 (<class 'numpy.float64'>)
0.5 (<class 'float'>) -> 0.5 (<class 'numpy.longdouble'>)

Output with this PR:

<module 'numpy' from '/mnt/data/numpy/numpy/__init__.py'> 2.0.0.dev0+git20231107.a555e7d
0 (<class 'int'>) -> 0.1 (<class 'numpy.longdouble'>)
0 (<class 'int'>) -> 0.1 (<class 'numpy.longdouble'>)
1 (<class 'int'>) -> 0.7 (<class 'numpy.longdouble'>)
1 (<class 'int'>) -> 0.7 (<class 'numpy.longdouble'>)
0.0 (<class 'float'>) -> 0.1 (<class 'numpy.longdouble'>)
0.0 (<class 'float'>) -> 0.1 (<class 'numpy.longdouble'>)
1.0 (<class 'float'>) -> 0.7 (<class 'numpy.longdouble'>)
1.0 (<class 'float'>) -> 0.7 (<class 'numpy.longdouble'>)
0.5 (<class 'float'>) -> 0.4 (<class 'numpy.longdouble'>)
0.5 (<class 'float'>) -> 0.4 (<class 'numpy.longdouble'>)
----
0 (<class 'int'>) -> 0 (<class 'numpy.int64'>)
0 (<class 'int'>) -> 0.0 (<class 'numpy.longdouble'>)
1 (<class 'int'>) -> 1 (<class 'numpy.int64'>)
1 (<class 'int'>) -> 1.0 (<class 'numpy.longdouble'>)
0.0 (<class 'float'>) -> 0.0 (<class 'numpy.float64'>)
0.0 (<class 'float'>) -> 0.0 (<class 'numpy.longdouble'>)
1.0 (<class 'float'>) -> 1.0 (<class 'numpy.float64'>)
1.0 (<class 'float'>) -> 1.0 (<class 'numpy.longdouble'>)
0.5 (<class 'float'>) -> 0.5 (<class 'numpy.float64'>)
0.5 (<class 'float'>) -> 0.5 (<class 'numpy.longdouble'>)

If you want I can add tests for this case as well.

@charris
Copy link
Member

charris commented Jan 4, 2024

@seberg ping.

@seberg
Copy link
Member

seberg commented Jan 30, 2024

Thanks @eendebakpt, sorry for forgetting to look over this again. I think it should be fine to do this cast and it seems like you gave it a spin with that test. So let's give it a shot.

@seberg seberg merged commit cae2234 into numpy:main Jan 30, 2024
60 checks passed
charris pushed a commit to charris/numpy that referenced this pull request Feb 3, 2024
* BUG: Fix np.quantile([Fraction(2,1)], 0.5)

* address review comments

* pass type instead of dtype instance
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Feb 3, 2024
charris added a commit that referenced this pull request Feb 3, 2024
BUG: Fix np.quantile([Fraction(2,1)], 0.5) (#24711)
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.

None yet

3 participants