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

[REVIEW] BUG Remove deprecated call to from_gpu_matrix #1198

Merged
merged 2 commits into from
Oct 10, 2020

Conversation

hlinsen
Copy link
Contributor

@hlinsen hlinsen commented Oct 7, 2020

This PR removes deprecated method DataFrame.from_gpu_matrix
Fixes: #1191

@hlinsen hlinsen requested a review from a team as a code owner October 7, 2020 18:06
@GPUtester
Copy link
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

Copy link
Member

@afender afender left a comment

Choose a reason for hiding this comment

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

👍

@afender
Copy link
Member

afender commented Oct 7, 2020

I've seen this test error on PR #1156 as well

@afender afender added this to the 0.16 milestone Oct 7, 2020
@BradReesWork
Copy link
Member

rerun tests

@afender
Copy link
Member

afender commented Oct 7, 2020

@raydouglass was there a recent change on the CI side that could cause all cugraph python tests to segfault in cudf (+ @kkraus14)?

Looking at cugraph merge history and cudf merge history of the last day I don't really see anything suspicious.

@kkraus14
Copy link
Contributor

kkraus14 commented Oct 7, 2020

@raydouglass was there a recent change on the CI side that could cause all cugraph python tests to segfault in cudf (+ @kkraus14)?

Looks to be CSV related:

 4  /opt/conda/envs/rapids/lib/python3.8/site-packages/cudf/_lib/csv.cpython-38-x86_64-linux-gnu.so(_ZNSt6vectorIN4cudf2io16column_name_infoESaIS2_EED1Ev+0xc4) [0x7f0901d69234]
 5  /opt/conda/envs/rapids/lib/python3.8/site-packages/cudf/_lib/csv.cpython-38-x86_64-linux-gnu.so(_ZN4cudf2io19table_with_metadataD1Ev+0xd6) [0x7f0901d6b0e6]
 6  /opt/conda/envs/rapids/lib/python3.8/site-packages/cudf/_lib/csv.cpython-38-x86_64-linux-gnu.so(+0x45460) [0x7f0901d51460]
 7  /opt/conda/envs/rapids/lib/python3.8/site-packages/cudf/_lib/csv.cpython-38-x86_64-linux-gnu.so(+0x4f657) [0x7f0901d5b657]

I know these APIs were under some churn recently, cc @vuule

@kkraus14
Copy link
Contributor

kkraus14 commented Oct 7, 2020

Also, looks like the libcudf and cudf builds that were build were mismatched which is a possible issue.

Would suggest adding libcudf / librmm here as well: https://github.com/rapidsai/cugraph/blob/branch-0.16/ci/gpu/build.sh#L60

This should help the conda solver prioritize getting the newest version of libcudf / librmm.

@afender
Copy link
Member

afender commented Oct 7, 2020

Quick updates:

I added the change to ci/gpu/build.sh#L60 in #1199 and still get the version mismatch :

cudf-0.16.0a201007         |cuda_11.0_py38_g8950c1a69e_1962        86.3 MB  rapidsai-nightly
libcudf-0.16.0a201006      |cuda11.0_g953f825b74_1958       242.5 MB  rapidsai-nightly

I did check cuML and they have the same mismatch. However, CI passes there.

@kkraus14
Copy link
Contributor

kkraus14 commented Oct 8, 2020

Quick updates:

I added the change to ci/gpu/build.sh#L60 in #1199 and still get the version mismatch :

cudf-0.16.0a201007         |cuda_11.0_py38_g8950c1a69e_1962        86.3 MB  rapidsai-nightly
libcudf-0.16.0a201006      |cuda11.0_g953f825b74_1958       242.5 MB  rapidsai-nightly

I did check cuML and they have the same mismatch. However, CI passes there.

They're likely not using the read_csv function which would have mismatched symbols. I'm guessing this is from us removing the dlpack dependency and subsequently adding it back in. I'm not sure how to fix this other than changing the way we pin the libcudf dependency in cudf to the exact build.

@BradReesWork
Copy link
Member

rerun tests

@BradReesWork
Copy link
Member

rerun tests

3 similar comments
@BradReesWork
Copy link
Member

rerun tests

@BradReesWork
Copy link
Member

rerun tests

@rlratzel
Copy link
Contributor

rlratzel commented Oct 9, 2020

rerun tests

@codecov-io
Copy link

codecov-io commented Oct 9, 2020

Codecov Report

Merging #1198 into branch-0.16 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           branch-0.16    #1198   +/-   ##
============================================
  Coverage        57.28%   57.28%           
============================================
  Files               61       61           
  Lines             2500     2500           
============================================
  Hits              1432     1432           
  Misses            1068     1068           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14c458...f608570. Read the comment docs.

@BradReesWork BradReesWork merged commit 3a3fbc6 into rapidsai:branch-0.16 Oct 10, 2020
@hlinsen hlinsen deleted the fa2-fix branch April 15, 2021 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Remove deprecated from_gpu_matrix from Force Atlas 2 wrapper
7 participants