feat: New quantile interpolation method & QUANTILE_DISC function in SQL

[pomo-mondreganto]

This is a followup of #18047. As suggested in the comment there, the current quantile interpolation methods are lacking, so this PR adds a new interpolation method that works as described here and uses the new method in QUANTILE_DISC SQL function. Conformance tests with DuckDB are included.

[pomo-mondreganto]

cc @alexander-beedie, let's continue here

[codecov]

Codecov Report

Attention: Patch coverage is 87.88927% with 35 lines in your changes missing coverage. Please review.

Project coverage is 80.01%. Comparing base (900dc3b) to head (ce001bb).
Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
.../polars-python/src/lazyframe/visitor/expr_nodes.rs	0.00%	7 Missing ⚠️
...ow/src/legacy/kernels/rolling/no_nulls/quantile.rs	79.16%	5 Missing ⚠️
crates/polars-sql/src/functions.rs	80.00%	4 Missing ⚠️
...arrow/src/legacy/kernels/rolling/nulls/quantile.rs	83.33%	3 Missing ⚠️
crates/polars-core/src/frame/group_by/mod.rs	0.00%	3 Missing ⚠️
...polars-core/src/chunked_array/ops/aggregate/mod.rs	97.82%	2 Missing ⚠️
...s-core/src/chunked_array/ops/aggregate/quantile.rs	94.87%	2 Missing ⚠️
crates/polars-core/src/frame/column/mod.rs	0.00%	2 Missing ⚠️
...s/polars-plan/src/dsl/functions/syntactic_sugar.rs	0.00%	2 Missing ⚠️
crates/polars-python/src/conversion/mod.rs	71.42%	2 Missing ⚠️
... and 3 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #19139      +/-   ##
==========================================
+ Coverage   79.99%   80.01%   +0.01%     
==========================================
  Files        1527     1527              
  Lines      209203   209138      -65     
  Branches     2415     2415              
==========================================
- Hits       167352   167334      -18     
+ Misses      41303    41256      -47     
  Partials      548      548              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[orlp]

Alright, correct me if I'm wrong, but (modulo implementation bugs), I believe our current interpolation methods all share the same properties:

1. They distribute the elements evenly among the number line [0, 1], with the first element at 0 and the last element at 1. This means that logically speaking, the ith element is found at position i / (n - 1).
2. If p == i / (n-1) exactly, we return the ith element unchanged. Otherwise we interpolate between the element before it and after it.

In this context our interpolation methods can be visualized as such, as an example for n = 4 elements:

This PR proposes to add another interpolation method which breaks the above convention. It does not distribute the elements among the number line [0, 1] starting at 0 and ending at 1, instead it chops up the number line [0, 1] into equal parts, assigning one value to each part in order:

Not shown is what to do about the boundaries, my preference would be left-closed if we had to choose.

Is my understanding correct? If so I have two issues with it:

1. I think the name Equiprobable is better than Bucket.
2. I'm not sure this should be an interpolation method instead of a different method altogether, as it does break our above properties that all the other interpolation methods share. If not a different method perhaps we should rename the 'interpolation' parameter to 'method'.

[pomo-mondreganto]

Yes, you're exactly right. I agree that Equiprobable is a better name, I'll update my PR. Regarding the second point, there're a few pieces of information that are relevant to this PR:

1. The main motivation for this PR is the previous discussion in feat: Quantile function in SQL #18047 regarding QUANTILE_DISC method. As far as I understand, Polars SQL is trying to be compatible with DuckDB's or PostgreSQL's implementation, and both use the equiprobable discrete quantiles under the hood.
2. I agree with @ soerenwolfers in the previous PR that the default expectation of discrete quantile for a set is that it returns each element with equal probability, and all current interpolation options are lacking this characteristic. I'd say that the default method should be either Linear or Equiprobable as other methods don't seem useful statistically. For example, I'm planning to use the proposed QUANTILE_DISC for metrics, and as I see it the best option for a "good enough" discrete quantile in my charts is Nearest, but I'd switch to Equilateral if this is merged.

Renaming to "Method" seems good enough (current discrete options like Lower or Higher are not exactly "interpolation" either), but that'd be a backward incompatible change and I don't know what's the Polars policy on that.

[orlp]

As long as it's just on the Rust side for now we don't have to worry about the renaming of interpolation to method just yet. And we can do such renames nicely with deprecation.

[pomo-mondreganto]

@orlp I've renamed the Options to Method, Bucket to Equiprobable and updated almost all variable names. The only case I didn't rename is the public-facing python bindings as someone could pass interpolation= as kwarg, which would break.

[pomo-mondreganto]

Is anything else needed here from me?

[orlp]

@pomo-mondreganto Looks good to me, @alexander-beedie can you review the SQL bits? Then we can merge this.

[alexander-beedie]

Looks good to me too; the SQL integration is sound, and the results tie out with other implementations. Very nice addition👌