Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated EXPM1() and Tests to New Engine #1334

Merged
merged 1 commit into from
Feb 14, 2023

Conversation

matthewryanwells
Copy link
Contributor

Signed-off-by: Matthew Wells matthew.wells@improving.com

Description

Updated EXPM1 function to run on the new engine, as well as added integration tests and unit tests. In addition also extracted common code that was repeated in many functions into a standalone base function they can call.

Issues Resolved

#1190

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

* Updated EXPM1() and tests to new engine

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Added EXPM1 to PPL Parser

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Added documentation for EXPM1 PPL and fixed small mistake in EXP documentation

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Added newline to fix code style issue

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* removed unnecessary integration tests and fixed typo in another test

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Extracted common code from math functions into external function

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Added javadoc comment to new function

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Improved Javadoc

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Added and improved EXPM1 tests, and improved documentation for CLI and PPL

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Removed EXPM1 from PPL

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Updated math base function to take any type and included more functions to use the base

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Made tiny change to documentation

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* reverted euler function to previous iteration as the one I made didn't work

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* fixed typo in a comment that I made in a previous commit

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* fixed a mistake I made in fixing the typo from my last commit

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Fixed spacing inconsistencies in math.rst file

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* removed the base math function so that it can be pushed in a seperate pull request

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* Reverted reverted change that removes the refactoring as this will be done in one commit

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

* updated base function to have more descriptive variables

Signed-off-by: Matthew Wells <matthew.wells@improving.com>

---------

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Copy link
Collaborator

@dai-chen dai-chen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes! Shall we send separate PR for PPL?

@acarbonetto
Copy link
Collaborator

Thanks for the changes! Shall we send separate PR for PPL?

Seems like a low priority. We can raise a separate issue to create PPL equivalents.

@dai-chen
Copy link
Collaborator

Thanks for the changes! Shall we send separate PR for PPL?

Seems like a low priority. We can raise a separate issue to create PPL equivalents.

I wonder is this function for PPL low priority or having same scalar function set between SQL and PPL low priority?

@Yury-Fridlyand
Copy link
Collaborator

This function was migrated from the legacy engine. It is pretty useless though, because expm1(x) === exp(x) - 1.

@acarbonetto
Copy link
Collaborator

@dai-chen #1339 raised to address PPL. So this one is unblocked.

@Yury-Fridlyand Yury-Fridlyand merged commit e990201 into opensearch-project:main Feb 14, 2023
@Yury-Fridlyand Yury-Fridlyand deleted the integ-expm1 branch February 14, 2023 19:06
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 16, 2023
* Updated EXPM1() and tests to new engine

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
(cherry picked from commit e990201)
penghuo pushed a commit that referenced this pull request Feb 16, 2023
* Updated EXPM1() and tests to new engine

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
(cherry picked from commit e990201)

Co-authored-by: Matthew Wells <matthew.wells@improving.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants