Add numpy like percentile and quantile#942
Add numpy like percentile and quantile#942Dhruvanshu-Joshi wants to merge 4 commits intopymc-devs:mainfrom
Conversation
658e615 to
cc8d4d9
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #942 +/- ##
==========================================
+ Coverage 81.38% 81.61% +0.22%
==========================================
Files 172 178 +6
Lines 46868 47285 +417
Branches 11423 11497 +74
==========================================
+ Hits 38145 38592 +447
+ Misses 6542 6504 -38
- Partials 2181 2189 +8
|
cc8d4d9 to
77d8604
Compare
pytensor/tensor/math.py
Outdated
|
|
||
| percentiles = [100.0 * x for x in q] | ||
|
|
||
| return percentile(input, percentiles, axis) |
There was a problem hiding this comment.
This is a but irrelevant, but does it make sense to define quantile first and then percentile from quantile. So the other way around?
Quantile seems more fundamental
There was a problem hiding this comment.
Yes makes sense. I have pushed this.
14967fe to
c3bda8a
Compare
pytensor/tensor/math.py
Outdated
| for quantile in q: | ||
| if quantile < 0 or quantile > 1: | ||
| raise ValueError("Quantiles must be in the range [0, 1]") | ||
|
|
||
| result = [] | ||
| for quantile in q: | ||
| k = (quantile) * (input_shape - 1) | ||
| k_floor = floor(k).astype("int64") | ||
| k_ceil = ceil(k).astype("int64") | ||
|
|
||
| slices1[-1] = slice(k_floor, k_floor + 1) | ||
| slices2[-1] = slice(k_ceil, k_ceil + 1) | ||
| val1 = sorted_input[tuple(slices1)] | ||
| val2 = sorted_input[tuple(slices2)] | ||
|
|
||
| d = k - k_floor | ||
| quantile_val = val1 + d * (val2 - val1) | ||
|
|
||
| result.append(quantile_val.squeeze(axis=-1)) |
There was a problem hiding this comment.
This won't work with symbolic q. Can we have a vectorized implementation?
There was a problem hiding this comment.
Hi. Sorry this completely skipped me. I'll get back to it and see how to vectorise this and also look into the failing tests.
|
This code could look a lot like the median one, with the exception that the k in We should be able to re-implement the median as |
c3bda8a to
dcdeef7
Compare
|
Hi @ricardoV94 I have added some changes. However I need some help in some parts. For "ndim, axis, q" equal to (3, (1, 2), [0.1, 0.9]), the tests fail due to some shapes issues: Also, I am confused on how to introduce symbolic checks to ensure the all of the q is between 0 to 1 in case of quantile and 0 to 100 in case of percentile. |
We can be a bit permissive and consider it undefined behavior if it makes our code simpler. Otherwise there's a |
I'll try to take a look when I get some free time |
Description
This pr aims to add support for
numpylike percentile and quantile functions.Related Issue
Checklist
Type of change