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

[BUG] Build RAFT tests with -Wall -Werror #225

Closed
trxcllnt opened this issue May 11, 2021 · 3 comments
Closed

[BUG] Build RAFT tests with -Wall -Werror #225

trxcllnt opened this issue May 11, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@trxcllnt
Copy link
Collaborator

As a header-only library, warnings in RAFT can cause errors in consuming libs that compile with -Werror. Building tests with -Wall -Werror like RMM does should catch most problems before they're released.

For example, see this error in a RAFT header when compiling cuML:

raft/cpp/include/raft/sparse/hierarchy/detail/mst.cuh: In instantiation of 'void raft::hierarchy::detail::build_sorted_mst(const raft::handle_t&, const value_t*, const value_idx*, const value_idx*, const value_t*, size_t, size_t, rmm::device_uvector<value_idx>&, rmm::device_uvector<value_idx>&, rmm::device_uvector<value_t>&, size_t, raft::distance::DistanceType, int) [with value_idx = int; value_t = float; size_t = long unsigned int]':
raft/cpp/include/raft/sparse/hierarchy/single_linkage.hpp:80:47:   required from 'void raft::hierarchy::single_linkage(const raft::handle_t&, const value_t*, size_t, size_t, raft::distance::DistanceType, raft::hierarchy::linkage_output<value_idx, value_t>*, int, size_t) [with value_idx = int; value_t = float; raft::hierarchy::LinkageDistance dist_type = raft::hierarchy::PAIRWISE; size_t = long unsigned int]'
cuml/cpp/src/hierarchy/linkage.cu:32:117:   required from here
raft/cpp/include/raft/sparse/hierarchy/detail/mst.cuh:204:410: warning: format '%d' expects argument of type 'int', but argument 4 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
@trxcllnt trxcllnt added the feature request New feature or request label May 11, 2021
@trxcllnt
Copy link
Collaborator Author

I started this as a feature-request, but realized it's better as a bug. That's why the label is wrong.

@dantegd dantegd added bug Something isn't working and removed feature request New feature or request labels May 12, 2021
@dantegd
Copy link
Member

dantegd commented May 13, 2021

PR #187 adds the cmake side to enable this easily, was going to push it in the same PR but (as usual) once I fix what it looked like a single warning, more started popping up, so it probably needs to be a follow up.

rapids-bot bot pushed a commit that referenced this issue Jul 28, 2021
This PR fixes current RAFT C++/CUDA compilation warnings and turns on -Wall to treat warnings as errors.

Fixes #225
Fixes #289

Authors:
  - Mark Harris (https://github.com/harrism)

Approvers:
  - Brad Rees (https://github.com/BradReesWork)
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #299
@dantegd
Copy link
Member

dantegd commented Jul 30, 2021

Closed nu #299

@dantegd dantegd closed this as completed Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants