ARROW-11070: [C++][Compute] Implement power kernel

[rok]

This is to resolve ARROW-11070.

[rok]

I'm not sure about two things:

• Is Exponentiate a good name or should we go with Power? I picked Exponentiate as all the other operation names are verbs so Power feels inconsistent.
• Should we add overflow checking logic?
• What to do with nulls in case of power(null, 0) or power(1, null). See comment

[cyb70289]

I'm not sure about two things:

* Is `Exponentiate` a good name or should we go with `Power`? I picked `Exponentiate` as all the other operation names are verbs so `Power` feels inconsistent.

I prefer power which is used widely. But I'm bad at naming, so would like to hear other comments.

* Should we add overflow checking logic?

I don't think so. Let it be infinity looks okay.
[EDIT] We do not need to check floating point overflow. But I think it's necessary for integers, like other arithmetic kernel does.

PS: Numpy checks floating point overflow, but not integer overflow. Looks strange to me.

In [2]: np.power(10, 1000)
Out[2]: 0
In [3]: np.power(10.0, 1000)
<ipython-input-20-202eedf3dc47>:1: RuntimeWarning: overflow encountered in power
np.power(10.0, 1000)
Out[3]: inf

[github-actions]

https://issues.apache.org/jira/browse/ARROW-11070

[rok]

Thanks for the feedback @cyb70289. I'll do another commit tomorrow and kindly ask for feedback after.

[rok]

@cyb70289 Could you please have another look?
I've added the docs and some tests.
As for 0 raised to negative integers I'm leaning more towards the way R/C++ way as it is more general mathematically and can be de-generalized in other layers.

[rok]

CI issues don't seem to be related.

[rok]

Should we also add a benchmark? I see other kernels have them.

[cyb70289]

Should we also add a benchmark? I see other kernels have them.

Yes we should. It can be in another pr.
Thanks for persistent in improving this pr. Will review soon.

[rok]

Should we also add a benchmark? I see other kernels have them.

Yes we should. It can be in another pr.

I've added the benchmark. I hope I've not missed something there.

Thanks for persistent in improving this pr. Will review soon.

Thanks for the feedback! :)

[rok]

I had to reduce the range to avoid overflows. Would it make more sense to introduce a separate generator for power benchmark only?

[cyb70289]

I tweaked some code and removed the benchmark.
For now, I think there are still many things needs to improve. So we don't need to add benchmark in a haste.
I downloaded and attached your benchmark commit, in case it's lost.
0001-Adding-benchmark-for-Power-and-PowerChecked.patch.gz

[nealrichardson]

@cyb70289 is this good to merge now? (The RTools35 CI failure has been fixed on master)

[cyb70289]

Will merge when CI is green.

[jorisvandenbossche]

I added a few comments on the docstring of the function. But feel free to merge when green, and this can also be done in a follow-up.

[cyb70289]

Record things to improve, will create Jira to track.

Power kernel is modified from add/sub/mul/div kernels. But power is much more complicated than basic arithmetic operations. Some limitations in current implementation:

• Base and exponent must be the same type, this is not reasonable
• Null handling is delayed to the end, and power is always calculated even for nulls (with whatever base/exponent happen to be in the slots). This is a big waste of time for expensive power calculations.

For checked power, integer overflow is checked for every multiplication. As the integer upper bound is already know, it can be calculate directly if the power will overflow. It will remove overflow checks in each iteration, but will introduce one expensive logarithm operation. Maybe deserve a try.

[nealrichardson]

CI failure is unrelated, will merge this. @rok please link on here the followup JIRA when you make it.

[cyb70289]

Created jira task to track improving power kernel: https://issues.apache.org/jira/browse/ARROW-12413