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

Fix gtest column utility comparator diff reporting #12995

Merged
merged 5 commits into from
Mar 24, 2023

Conversation

davidwendt
Copy link
Contributor

Description

Fixes a bug introduced in #12777 (by me) in column_utilities.cu that caused the difference reporting in a failed comparison in a gtest result (i.e. through CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT for example) to report the incorrect row.
The logic was changed to improve compile time but incorrectly created indices of only 0s and 1s in the difference vector.
This PR fixes the logic to create the correct indices for the reporting logic.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt added bug Something isn't working 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Mar 22, 2023
@davidwendt davidwendt requested a review from a team as a code owner March 22, 2023 22:24
@davidwendt davidwendt self-assigned this Mar 22, 2023
@ttnghia
Copy link
Contributor

ttnghia commented Mar 23, 2023

Just hit tested failures caused by the issue that is fixed by this 👍 :

Failed
first difference: lhs[4] = 0, rhs[4] = 0

@davidwendt
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit ba8116a into rapidsai:branch-23.04 Mar 24, 2023
@davidwendt davidwendt deleted the bug-column-utilities branch March 24, 2023 21:01
rapids-bot bot pushed a commit that referenced this pull request Apr 10, 2023
This fixes the lexicographic comparator that cannot handle the input having structs of lists. The new implementation mainly changes the helper functions `decompose_structs`. In particular:
 * If a structs column has its first child is a lists column, the first column of the result table will no longer be `Struct<Struct<...<List<SomeType>...>` (i.e., nested structs ultimately having one child).
 * Instead, the first output column will be nested empty structs: `Struct<...Struct<>>...>`. The innermost child column `List<SomeType>` is output as the second column in the result table.

Depends on:
 * #12995

Closes #11672.

Authors:
  - Nghia Truong (https://github.com/ttnghia)
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Divye Gala (https://github.com/divyegala)
  - Bradley Dice (https://github.com/bdice)

URL: #13005
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants