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.

Sign up for GitHub

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

Jump to bottom

[REVIEW] Change ColumnViewAccess usage to work with ColumnView #1105

Merged
merged 3 commits into from
Nov 18, 2020
Merged

[REVIEW] Change ColumnViewAccess usage to work with ColumnView #1105

merged 3 commits into from
Nov 18, 2020

Conversation

kuhushukla
Copy link
Collaborator

Blocks on rapidsai/cudf#6751 going in cudf. If we are ok with exposing the columnView of a ColumnVector then I can remove some of the redundant code I had to add to make it work, since now ColumnVector and ColumnView dont have any shared hierarchy.

@kuhushukla kuhushukla added the P0 Must have for release label Nov 12, 2020
@kuhushukla kuhushukla added this to the Nov 9 - Nov 20 milestone Nov 12, 2020
kuhushukla
kuhushukla commented Nov 12, 2020
View reviewed changes
sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java
}
}

private static boolean typeConversionAllowed(ColumnView cv, DataType colType) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we are ok with exposing the columnView of a ColumnVector then I can remove some of the redundant code I had to add to make it work, since now ColumnVector and ColumnView dont have any shared hierarchy.

@kuhushukla
Copy link
Collaborator Author

Just rebased it on top of the cudf change, taking the redundant code out.

@kuhushukla kuhushukla changed the title [WIP] Change ColumnViewAccess usage to work with ColumnView [REVIEW] Change ColumnViewAccess usage to work with ColumnView Nov 18, 2020
@kuhushukla
Copy link
Collaborator Author

rapidsai/cudf#6751 is in and I have moved it to review state, @revans2 , @jlowe Please take a look.

jlowe
jlowe approved these changes Nov 18, 2020
View reviewed changes
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

Since this is a knife-switch to the API changes in cudf, premerge CI won't pass until we publish a new cudf 0.17-SNAPSHOT with the API changes. I'd rather not force that early and break every premerge CI until we know this is absolutely ready to go. Have you manually run unit and integration tests against the new cudf? If so we'll kick the cudf build now and get this in ASAP after the snapshot is published.

@kuhushukla
Copy link
Collaborator Author

@jlowe agree, I had run the unit tests/ scala tests which pass and running python integration tests to be sure, which is taking a bit of effort on my local machine due to overheating. I will post here once done.

@kuhushukla
Copy link
Collaborator Author

Python integration tests are passing save the ones we already know about AFAIK

FAILED src/main/python/orc_write_test.py::test_buckets_write_fallback[ALLOW_NON_GPU(DataWritingCommandExec)]
FAILED src/main/python/parquet_write_test.py::test_buckets_write_fallback[ALLOW_NON_GPU(DataWritingCommandExec)]
= 2 failed, 3484 passed, 202 skipped, 188 xfailed, 1 warning in 3873.17s (1:04:33) =

@jlowe
Copy link
Member

jlowe commented Nov 18, 2020

I've kicked the cudf snapshot build manually. This will need to be upmerged to latest branch-0.3 to get those other two unit tests to pass premerge CI.

@kuhushukla
Copy link
Collaborator Author

Thanks Jason. I have upmerged and started the ci.

@kuhushukla
Copy link
Collaborator Author

build

1 similar comment
@kuhushukla
Copy link
Collaborator Author

build

@kuhushukla kuhushukla self-assigned this Nov 18, 2020
@revans2 revans2 merged commit de005d3 into NVIDIA:branch-0.3 Nov 18, 2020
sperlingxx pushed a commit to sperlingxx/spark-rapids that referenced this pull request Nov 20, 2020
@sperlingxx
Change ColumnViewAccess usage to work with ColumnView (NVIDIA#1105)
c776062 
Signed-off-by: Kuhu Shukla <kuhus@nvidia.com>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
Change ColumnViewAccess usage to work with ColumnView (NVIDIA#1105)
df6ec57 
Signed-off-by: Kuhu Shukla <kuhus@nvidia.com>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
Change ColumnViewAccess usage to work with ColumnView (NVIDIA#1105)
4a4d312 
Signed-off-by: Kuhu Shukla <kuhus@nvidia.com>
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
@nvauto
Update submodule cudf to ca2ca37001772ef179e72d94a1824cea1b7b62e6 (NV…
7bd892c 
…IDIA#1105)

Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlowe jlowe jlowe approved these changes

@revans2 revans2 revans2 approved these changes

Assignees

@kuhushukla kuhushukla

Labels
P0 Must have for release
Projects
None yet
Milestone
Nov 9 - Nov 20
Development

Successfully merging this pull request may close these issues.
3 participants
@kuhushukla @jlowe @revans2