-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BUG: quantile
should error when weights
are all zeros
#28589
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
Comments
Thanks for reporting! Interesting that it ends up with something after giving an invalid warning. CC @lorentzenchr, but I suspect this shouldn't be hard to fix by anyone, probably really needs an explicit check. (Ah, I suppose the weights end up |
Would love to work on this issue! |
We don't assign issues, please just work on a PR with tests. |
Maybe, this should be aligned with |
I also found this weird behavior in
it outputs:
Also,
outputs:
|
…eights are all zeros numpy#28589 Previously, np.quantile with all weights set to zero returned the first sample. This is fixed now via adding an explicit check that raises a ValueError when all weights are zero, preventing division by zero in the CDF. - Added the weight check in `_quantile` (line 4874 ff) to detect all-zero weights and raise an error. - Added a test in `test_function_base.py` to verify this behavior. - Added a second test to ensure existing quantile functionality remains unchanged. This should close numpy#28589
Linking related issue: #21091 Not sure what the right behaviour is when a weight is nan, potentially ignore that sample like with |
We now have three open pull requests to close this issue (#28594, #28595, and #28597), all coming in within a day or so. We should probably close two of them. I'd appreciate it if the contributors who opened pull requests for this could figure out which one we should review. Also, before opening a pull request, please double-check that there isn't an open PR already. |
I don't think we need to handle nan weights. For all zero weights, it should be identical to quantile over no points |
CC @rottenstea , @Tontonio3, and @galio1237 can you have a look at #28589 (comment)? I think all of these PRs need some work/thought still. I don't want to review all 3 and some got helpful comments already, so I'll make some broad comments here:
(If I would trust compilers/systems, all we actually would need to do is add a |
I have to raise concerns against this statement. Please do not restict weights in any way for For quantiles, it can be argued, things are different because an empirical CDF is calculated based on the weights which are therefore required to be non-negative and to sum to a positive number. |
Cool, thanks for pointing that out, sorry, I think there were more functions then just percentile/quantile that check |
There is a test in .\numpy\lib\tests\test_function_base.py that has a That's why I opened issue #28602 |
Describe the issue:
np.quantile
withweights
being all zeros should probably give a error/warning. It's effectively like asking for a quantile of an empty array. Currently numpy seems to return the first sample.Reproduce the code example:
Error message:
Python and NumPy Versions:
2.1.3
3.13.0 | packaged by conda-forge | (main, Nov 27 2024, 19:18:50) [GCC 13.3.0]
Runtime Environment:
No response
Context for the issue:
Context: Noticed when looking into scikit-learn/scikit-learn#31032
The text was updated successfully, but these errors were encountered: