velox
velox copied to clipboard
facebookincubator
Add the math function expm1
Summay:
Levarage the std::expm1 to implement the function of expm1 and register it in sparksql Fix #9716
Hi @Donvi!
Thank you for your pull request and welcome to our community.
Action Required
In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.
Process
In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.
Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.
If you have received this in error or have any questions, please contact us at [email protected]. Thanks!
Deploy Preview for meta-velox canceled.
| Name | Link |
|---|---|
| Latest commit | 1cfcd7b5d9dfd83628040ff8585878e8ca675698 |
| Latest deploy log | https://app.netlify.com/sites/meta-velox/deploys/6655ad9ecfca2c0008f839fd |
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!
Would you document this function and add some unit tests? Thanks.
This is for unsupported function expm1, which is more accurate than the expression std::exp(num) - 1.0 if num is close to zero. And the c++ std provide the function std::expm1, which can be referenced here. UT has been added.
@Donvi I'm trying to say maybe add some documentation for the added Velox function in https://github.com/facebookincubator/velox/tree/main/velox/docs/functions/spark.
@PHILO-HE thank you for reviewing and fixing, any further comments?
@Donvi, looks good! @mbasmanova, could you take a look?
@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@pedroerp All comments are resolved, so is there any further action needed?
thank you @Donvi . I'm triggering the CI jobs again
@pedroerp merged this pull request in facebookincubator/velox@b7bd2a44dd4256868dd5612509cabdc904d78dd8.
Conbench analyzed the 1 benchmark run on commit b7bd2a44.
There were no benchmark performance regressions. 🎉
The full Conbench report has more details.
![conbench-facebook[bot] avatar](https://avatars.githubusercontent.com/in/241235?v=4 "Published by a pub.dev verified publisher"){kind=small}