[FIX] Fix build with latest RAFT branch-21.08 #4012

trxcllnt · 2021-06-24T01:41:32Z

These things are breaking cuML builds with the latest raft branch-21.08. Blocked until rapidsai/raft#279 is resolved.

cpp/src/metrics/pairwise_distance.cu

cpp/src/tsne/distances.cuh

cpp/src/umap/knn_graph/algo.cuh

robertmaynard

CMake changes LGTM

dantegd · 2021-06-24T15:41:12Z

@trxcllnt: CI will fail without the (really small) change needed for numpy 1.21: #4008, could you apply the fix here (I'll go ahead and close 4008)

…aft-knn-changes

trxcllnt · 2021-06-24T16:58:47Z

@dantegd sure thing, pulled

cjnolet · 2021-06-24T20:04:13Z

@trxcllnt in the past we have locked cuml into a specific raft commit hash so that updates to raft don't break cuml. It looks like the pinned tag is being set automatically in cuml now which means changes to raft are now strongly coupled to cuml. Do you think there's a way we could decouple these two and sync them back at release time? This would allow us to make updates to raft and update the cuml side intentionally (without breaking everyone else).

cjnolet

Changes look good. I would like to update the sparse bfknn API on the raft side to make the handle const. It's a super simple change and I can file a PR for it if you prefer.

cjnolet · 2021-06-24T20:13:01Z

cpp/src/tsne/distances.cuh

-    handle.get_cusparse_handle(), handle.get_device_allocator(), stream,
-    ML::Sparse::DEFAULT_BATCH_SIZE, ML::Sparse::DEFAULT_BATCH_SIZE,
-    DEFAULT_DISTANCE_METRIC);
+    const_cast<raft::handle_t &>(handle), ML::Sparse::DEFAULT_BATCH_SIZE,


Oh boy. I don't know how I missed this one- that should be a const on the raft side for sure.

cjnolet · 2021-06-24T20:13:12Z

cpp/src/umap/knn_graph/algo.cuh

-    handle.get_cusparse_handle(), d_alloc, stream,
-    ML::Sparse::DEFAULT_BATCH_SIZE, ML::Sparse::DEFAULT_BATCH_SIZE,
-    raft::distance::DistanceType::L2Expanded);
+    const_cast<raft::handle_t &>(handle), ML::Sparse::DEFAULT_BATCH_SIZE,


This one as well.

cjnolet · 2021-06-24T21:48:33Z

@trxcllnt I opened rapidsai/raft#280 to add the const qualifier to the raft handle in the sparse bfknn

trxcllnt · 2021-06-24T22:19:37Z

@cjnolet cuML and cuGraph using separate commits isn't compatible with using a shared build cache via CMake's FETCHCONTENT_BASE_DIR. We're hoping to use this strategy to speed up local and CI builds soon (and have been using it in node-rapids), since all the duplicated dependencies can be built once in a common location and reused for the next project that declares them.

For example, cuML and cuGraph both declare a dependency on faiss. It's built as a byproduct of cuML, and cuGraph can find it already built in the common build location. The common build dir path can also be stable, enabling new clean-builds of faiss to be sped up via ccache (or sccache).

This relies on the two using the same version of any common dependencies, since FETCHCONTENT_BASE_DIR can only be set globally, not for individual dependencies. It's an all-or-nothing approach unfortunately.

cjnolet

LGTM

cjnolet · 2021-06-25T02:53:17Z

rerun tests

dantegd · 2021-06-25T12:11:38Z

Argh it seems I missed this instance of the abs NumPy call that requires an update:

cuml/python/cuml/test/dask/test_dbscan.py

Line 91 in 76e6b51

X_dist = np.abs(Xc - Xc.T, dtype=datatype)

python/cuml/test/test_dbscan.py

trxcllnt · 2021-06-25T16:47:16Z

rerun tests

cjnolet · 2021-06-25T21:03:31Z

@gpucibot merge

levsnv · 2021-06-26T00:07:59Z

same failures:

FAILED cuml/test/dask/test_dbscan.py::test_dbscan_precomputed[int32-500-float32-1]
FAILED cuml/test/dask/test_dbscan.py::test_dbscan_precomputed[int64-500-float32-1]

rerun tests

cjnolet · 2021-06-26T00:20:53Z

@levsnv i think the CI wasn't running the most recent code because the trace in the test failure is no longer in @trxcllnt's branch. Wondering if there was a hiccup in CI and it didn't properly pre-empt a previous execution from "rerun tests"

cjnolet · 2021-06-26T15:26:44Z

@levsnv @trxcllnt I see what happened now. The same changes to the precomputed test also need to be made to the Dask test. It should be copy/paste of those 2 updates to the non-dask test.

…anges

ajschmidt8 · 2021-06-28T15:38:27Z

hidden in the log output is the following error. I'm experiencing this locally when trying to build cuml as well.

trxcllnt · 2021-06-28T15:45:53Z

rerun tests

codecov-commenter · 2021-06-29T00:57:43Z

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.08@ac37465). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-21.08    #4012   +/-   ##
===============================================
  Coverage                ?   85.44%           
===============================================
  Files                   ?      230           
  Lines                   ?    18088           
  Branches                ?        0           
===============================================
  Hits                    ?    15455           
  Misses                  ?     2633           
  Partials                ?        0

Flag	Coverage Δ
dask	`48.04% <0.00%> (?)`
non-dask	`77.79% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

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 ac37465...1a760ed. Read the comment docs.

dantegd · 2021-06-29T01:19:36Z

@gpucibot merge

These things are breaking cuML builds with the latest raft `branch-21.08`. Blocked until rapidsai/raft#279 is resolved. Authors: - Paul Taylor (https://github.com/trxcllnt) - Corey J. Nolet (https://github.com/cjnolet) - Dante Gama Dessavre (https://github.com/dantegd) Approvers: - Robert Maynard (https://github.com/robertmaynard) - Corey J. Nolet (https://github.com/cjnolet) URL: rapidsai#4012

cjnolet and others added 3 commits June 23, 2021 13:15

Skipping hdbscan pytests when gpu is a100 for noe

6ca8487

FIX Update abs call for np 1.21 change

fc20d8a

update to latest RAFT changes

8b6b7c1

trxcllnt requested review from a team as code owners June 24, 2021 01:41

github-actions bot added CMake CUDA/C++ labels Jun 24, 2021

trxcllnt changed the title ~~Update to latest RAFT changes~~ [FIX] Fix build with latest RAFT branch-21.08 Jun 24, 2021

trxcllnt commented Jun 24, 2021

View reviewed changes

cpp/src/metrics/pairwise_distance.cu Outdated Show resolved Hide resolved

trxcllnt commented Jun 24, 2021

View reviewed changes

cpp/src/tsne/distances.cuh Outdated Show resolved Hide resolved

trxcllnt commented Jun 24, 2021

View reviewed changes

cpp/src/umap/knn_graph/algo.cuh Outdated Show resolved Hide resolved

fix lint

1799349

ajschmidt8 added non-breaking Non-breaking change bug Something isn't working labels Jun 24, 2021

robertmaynard approved these changes Jun 24, 2021

View reviewed changes

Merge branch 'fix-pytest-np121' of github.com:dantegd/cuml into fix/r…

fd209b7

…aft-knn-changes

trxcllnt requested a review from a team as a code owner June 24, 2021 16:58

github-actions bot added the Cython / Python Cython or Python issue label Jun 24, 2021

cjnolet requested changes Jun 24, 2021

View reviewed changes

cjnolet mentioned this pull request Jun 24, 2021

[REVIEW] Add chebyshev, canberra, minkowksi and hellinger distance metrics rapidsai/raft#276

Merged

remove const_cast in anticipation of rapidsai/raft#280

402da8e

cjnolet approved these changes Jun 24, 2021

View reviewed changes

cjnolet reviewed Jun 25, 2021

View reviewed changes

python/cuml/test/test_dbscan.py Outdated Show resolved Hide resolved

use square matrix of precomputed distances in test_dbscan_precomputed

fb41f2e

cjnolet added 2 commits June 26, 2021 11:39

Fixing dbscan test

fc24291

Merge branch 'bug-2108-skip_hdbscan_pytest_a100' into fix/raft-knn-ch…

098c3f8

…anges

trxcllnt mentioned this pull request Jun 28, 2021

Only add cuco::cuco alias to global_targets rapidsai/raft#282

Closed

FIX correct cuml exports instruction

1a760ed

levsnv mentioned this pull request Jun 29, 2021

Improve FIL code readability and documentation #3056

Merged

rapids-bot bot merged commit f71d369 into rapidsai:branch-21.08 Jun 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FIX] Fix build with latest RAFT branch-21.08 #4012

[FIX] Fix build with latest RAFT branch-21.08 #4012

trxcllnt commented Jun 24, 2021

robertmaynard left a comment

dantegd commented Jun 24, 2021

trxcllnt commented Jun 24, 2021

cjnolet commented Jun 24, 2021

cjnolet left a comment

cjnolet Jun 24, 2021

cjnolet Jun 24, 2021

cjnolet commented Jun 24, 2021

trxcllnt commented Jun 24, 2021

cjnolet left a comment

cjnolet commented Jun 25, 2021

dantegd commented Jun 25, 2021

trxcllnt commented Jun 25, 2021

cjnolet commented Jun 25, 2021

levsnv commented Jun 26, 2021

cjnolet commented Jun 26, 2021

cjnolet commented Jun 26, 2021

ajschmidt8 commented Jun 28, 2021

trxcllnt commented Jun 28, 2021

codecov-commenter commented Jun 29, 2021

dantegd commented Jun 29, 2021

[FIX] Fix build with latest RAFT branch-21.08 #4012

[FIX] Fix build with latest RAFT branch-21.08 #4012

Conversation

trxcllnt commented Jun 24, 2021

robertmaynard left a comment

Choose a reason for hiding this comment

dantegd commented Jun 24, 2021

trxcllnt commented Jun 24, 2021

cjnolet commented Jun 24, 2021

cjnolet left a comment

Choose a reason for hiding this comment

cjnolet Jun 24, 2021

Choose a reason for hiding this comment

cjnolet Jun 24, 2021

Choose a reason for hiding this comment

cjnolet commented Jun 24, 2021

trxcllnt commented Jun 24, 2021

cjnolet left a comment

Choose a reason for hiding this comment

cjnolet commented Jun 25, 2021

dantegd commented Jun 25, 2021

trxcllnt commented Jun 25, 2021

cjnolet commented Jun 25, 2021

levsnv commented Jun 26, 2021

cjnolet commented Jun 26, 2021

cjnolet commented Jun 26, 2021

ajschmidt8 commented Jun 28, 2021

trxcllnt commented Jun 28, 2021

codecov-commenter commented Jun 29, 2021

Codecov Report

dantegd commented Jun 29, 2021