GH-45190: [C++][Compute] Add rank_quantile function

[pitrou]

Rationale for this change

Add a "rank_quantile" function following the Wikipedia definition:
https://en.wikipedia.org/wiki/Percentile_rank

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes, an additional compute function.

• GitHub Issue: [C++][Compute] Add percentile rank function #45190

[pitrou]

@zanmato1984 @jorisvandenbossche Do you want to take a look at this?

Naming-wise, I hesitated between "rank_percentile" and "percentile_rank". I chose the former for better tab-completion.

[zanmato1984]

If the factor is free to specify, is the name rank_quantile more accurate? (BTW I prefer rank_xxx to xxx_rank).

Update 1: Ah I just see factor only affects the output scale, so it doesn't change the sense of "percentile". Please ignore my previous question.

Update 2: Oops, sorry I think my original question remains: if factor determines "what quantile" the output is, as implied by the comment of factor:

/// Use 1.0 for results in (0, 1), 100.0 for percentages, etc.

, then shall we name it rank_quantile? Or is rank_percentile so way more commonly used that no one will misinterpret it even though the factor can change the meaning of "percentile"?

[pitrou]

rank_quantile sounds ok to me. I find occurrences of "quantile rank" on Google, so it's not unheard of.

[zanmato1984]

Hi @pitrou , I want to suggest a refinement that simplifies the code structure of your current pr. I think the best way to show it is via actual code changes, so may I open up a pr targeting your branch? Thanks.

[pitrou]

Yes, you can!

[zanmato1984]

pitrou#13 filed. @pitrou PTAL.

[zanmato1984]

Seems the latest force push doesn't include the changes I've made. Anything wrong? :)

[pitrou]

Hmm, that's weird. I might have messed things up, let me take a look...

[pitrou]

Ok, this is fixed :)

[zanmato1984]

LGTM with two nits.

[zanmato1984]

This is good to go at any time.

[pitrou]

Yes, I was hoping for opinions on the general idea and API, but perhaps we can simply merge.

[WillAyd]

Sorry I did not comment sooner but I think the idea and API are great. If anything, it could be simpler to remove the factor and just leave it to the end user to specify that, but I don't think it's a big deal either way

[zanmato1984]

You can keep this open as long as you feel necessary for more ideas or comments. I was just saying I have no more to comment :)

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 17a0ff5.

There were 18 benchmark results indicating a performance regression:

• Commit Run on test-mac-arm at 2025-01-23 21:28:59Z
  • tpch (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-07, scale_factor=1
  • tpch (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-13, scale_factor=1
• and 16 more (see the report linked below)

The full Conbench report has more details. It also includes information about 491 possible false positives for unstable benchmarks that are known to sometimes produce them.