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

Add NaNs to Data Generators In Floating-Point Testing [databricks] #9334

Merged
merged 9 commits into from
Oct 2, 2023

Conversation

razajafri
Copy link
Collaborator

@razajafri razajafri commented Sep 28, 2023

This PR addresses an issue where our integration tests were not considering NaNs as part of floating-point values when testing various operators. NaN (Not-a-Number) is a critical value in floating-point arithmetic, and our tests should accurately reflect its behavior. We are addressing by removing *no_nans data generators in test files unless where they are necessary. e.g. versions of Spark that don't support NaN equality like in array_test.py

Changes Made:

  • Removed data generators that were excluding NaNs from floating-point values in test cases.
  • Updated affected test cases to include NaNs and ensure proper coverage for various operators.
  • Also renamed some of the variables to make a little bit more sense after this change.

Testing:

Ran the entire integration_test suite and saw no failures

fixes #8951

@sameerz sameerz added the test Only impacts tests label Sep 28, 2023
@kuhushukla
Copy link
Collaborator

@razajafri Add signoff but given the context I have, changes lgtm

@razajafri razajafri changed the title Remove Data Generators Ignoring NaNs in Floating-Point Testing Remove Data Generators Ignoring NaNs in Floating-Point Testing [databricks] Sep 29, 2023
@razajafri razajafri changed the title Remove Data Generators Ignoring NaNs in Floating-Point Testing [databricks] Add NaNs to Data Generators In Floating-Point Testing [databricks] Sep 29, 2023
@razajafri
Copy link
Collaborator Author

build

@jlowe jlowe merged commit 3ce1e9e into NVIDIA:branch-23.10 Oct 2, 2023
29 checks passed
@razajafri razajafri deleted the SP-8951-remove-no-nans branch October 2, 2023 15:26
@razajafri
Copy link
Collaborator Author

Thank you @jlowe for the review and merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Only impacts tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Tests are still using data generation with no NaNs
4 participants