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

[HOTFIX] Remove -g from cython compile commands #321

Merged
merged 41 commits into from
Sep 16, 2021
Merged

Conversation

ajschmidt8
Copy link
Member

Removes -g from the compile commands generated by distutils to compile Cython files.

This will make our container images, conda packages, and python wheels smaller.

raydouglass and others added 30 commits May 19, 2021 16:54
[gpuCI] Forward-merge branch-21.06 to branch-21.08 [skip ci]
[gpuCI] Forward-merge branch-21.06 to branch-21.08 [skip ci]
[gpuCI] Forward-merge branch-21.06 to branch-21.08 [skip ci]
[gpuCI] Forward-merge branch-21.06 to branch-21.08 [skip ci]
[gpuCI] Forward-merge branch-21.06 to branch-21.08 [skip ci]
[gpuCI] Forward-merge branch-21.06 to branch-21.08 [skip ci]
[gpuCI] Forward-merge branch-21.06 to branch-21.08 [skip ci]
[gpuCI] Forward-merge branch-21.06 to branch-21.08 [skip ci]
This RMM change has caused below build failure - rapidsai/rmm@7cbcd97 

[ 22%] Building CUDA object CMakeFiles/test_raft.dir/test/sparse/symmetrize.cu.o
/data/raft/cpp/test/mst.cu(202): error: no instance of constructor "rmm::device_buffer::device_buffer" matches the argument list
            argument types are: (int *, unsigned long)
          detected during instantiation of "void raft::mst::MSTTest<vertex_t, edge_t, weight_t>::SetUp() [with vertex_t=int, edge_t=int, weight_t=float]"
/data/raft/cpp/test/mst.cu(205): error: no instance of constructor "rmm::device_buffer::device_buffer" matches the argument list
            argument types are: (int *, unsigned long)
          detected during instantiation of "void raft::mst::MSTTest<vertex_t, edge_t, weight_t>::SetUp() [with vertex_t=int, edge_t=int, weight_t=float]"
/data/raft/cpp/test/mst.cu(208): error: no instance of constructor "rmm::device_buffer::device_buffer" matches the argument list
            argument types are: (float *, unsigned long)
          detected during instantiation of "void raft::mst::MSTTest<vertex_t, edge_t, weight_t>::SetUp() [with vertex_t=int, edge_t=int, weight_t=float]"
3 errors detected in the compilation of "/data/raft/cpp/test/mst.cu".
make[2]: *** [CMakeFiles/test_raft.dir/build.make:594: CMakeFiles/test_raft.dir/test/mst.cu.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:137: CMakeFiles/test_raft.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

Authors:
  - Mahesh Doijade (https://github.com/mdoijade)

Approvers:
  - Divye Gala (https://github.com/divyegala)
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #253
Updates raft's get_rmm.cmake logic to work like cuML and cuGraph, so that it works better with CalVer. This helps when building RAPIDS libraries locally and depending on a local RMM (e.g. with rapids-compose).

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

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #258
In comms/test.hpp `device_scalar::value` was not being passed an explicit stream, which means that the default stream was being synced. rapidsai/rmm#789 will remove the default from this parameter, and would have therefore broken the RAFT build. So this PR fixes the oversynchronization and ensures RAFT will build after the RMM PR is merged.

Note this PR includes the cmake changes from #258 (just so I could build locally). Once #258 is merged this PR's changes will be simplified.

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

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)
  - Chuck Hastings (https://github.com/ChuckHastings)

URL: #259
[gpuCI] Forward-merge branch-21.06 to branch-21.08 [skip ci]
[gpuCI] Forward-merge branch-21.06 to branch-21.08 [skip ci]
This PR reverts the pins that were made in #260. Those changes were only needed for the `21.06` branch.

Authors:
  - AJ Schmidt (https://github.com/ajschmidt8)

Approvers:
  - Peter Andreas Entschev (https://github.com/pentschev)
  - Dillon Cullinan (https://github.com/dillon-cullinan)

URL: #264
This PR addresses issues mentioned in #221
-- Adds grid stride based fusedL2NN kernel, this gives approx 1.85x speed up over previous version of this kernel.
-- Adds support in pairwise dist base class to work for any input size by adding support for grid stride based work distribution.

This was submitted to branch-21.06 through PR - #232 
but later reverted due to intermittent failure by - #246

Authors:
  - Mahesh Doijade (https://github.com/mdoijade)

Approvers:
  - Thejaswi. N. S (https://github.com/teju85)
  - Brad Rees (https://github.com/BradReesWork)

URL: #250
Per user request, this PR exposes the epsilon value that controls the precision where the Hungarian algorithm determines that a value has been reduced to sufficiently close to 0.

Authors:
  - Chuck Hastings (https://github.com/ChuckHastings)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)
  - Seunghwa Kang (https://github.com/seunghwak)

URL: #275
…eneration (#273)

Adds inline ptx assembly for lds & sts instructions for float, float2, float4, double, double2.
This ensures that compiler doesn't mistakenly generate non-vectorized instructions whenever we need it to generate vectorized version.
Also this ensures that we always generate non-generic ld/st instructions eliminating compiler from generating generic ld/st instructions.
These functions now requires the given shmem pointer should be aligned by the vector length, like for float4 lds/sts shmem pointer should be aligned by 16 bytes else it might silently fail or can also give runtime error.

Authors:
  - Mahesh Doijade (https://github.com/mdoijade)

Approvers:
  - Thejaswi. N. S (https://github.com/teju85)

URL: #273
Now that `rmm` uses `rapids-cmake` we need to update to the `21.08` branch to get the new `rapids_cmake_write_version_file` function

Authors:
  - Robert Maynard (https://github.com/robertmaynard)

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #278
This branch includes several new features and optimizations:

1. Introduces a hash table strategy to sparsify the vector in the coo spmv shared memory
2. Adds a batching strategy for rows with nnz too large to fit into shared memory
3. Removes the need for the cusparse csrgemm
4. Uses raft handle in distances_config_t rather than accepting each resource explicitly
5. Removes the naive CSR semiring code

This PR is also required to merge #261, which introduces the remaining distances

Authors:
  - Divye Gala (https://github.com/divyegala)
  - Corey J. Nolet (https://github.com/cjnolet)

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

URL: #269
cjnolet and others added 11 commits June 25, 2021 02:27
Safe to switch this back now that NVIDIA/cuCollections#90 is merged.

~edit: DO NOT MERGE until NVIDIA/cuCollections#91 is merged.~

edit 2: ready to merge again

Authors:
  - Paul Taylor (https://github.com/trxcllnt)

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #284
This change will be followed up by cuML side change to use these metrics and update the pytest accordingly.

Authors:
  - Mahesh Doijade (https://github.com/mdoijade)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #276
Always add the `FAISS::FAISS` library target alias if it doesn't exist. This can happen if cuML is built and installs FAISS before cuGraph or vice-versa.

Related PRs:
rapidsai/cuml#4028
rapidsai/cugraph#1694

Note: We can probably remove the `get_faiss.cmake` file in cuML and cuGraph since they both should get it from RAFT.

Authors:
  - Paul Taylor (https://github.com/trxcllnt)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #287
This PR is intended to be merged after #207 (hash table strategy) has been merged.

This PR introduces the following distances:
- Hamming
- Jensen-Shannon
- Russell-Rao
- KL-Divergence
- Correlation

Most of the changes here are from #207 and will be reviewed in that PR. The only files that need to be reviewed for this PR are `sparse/distance/l2_distance.cuh`, `sparse/distance/bin_distance.cuh`, `sparse/distance/lp_distances.cuh`, and their corresponding gtests: `test/sparse/distance.cuh`

Authors:
  - Corey J. Nolet (https://github.com/cjnolet)
  - Divye Gala (https://github.com/divyegala)

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #261
…nce & fusedL2NN kernels (#292)

overlap epilog compute with ldg of next grid stride in pairwise distance base class. 
gives 2-3% perf improvement for pairwise distance kernels and fusedL2NN kernel.

Authors:
  - Mahesh Doijade (https://github.com/mdoijade)

Approvers:
  - Thejaswi. N. S (https://github.com/teju85)

URL: #292
)

Closes #165 

Uses a C++17 `if constexpr` to discard at compile time a code path that doesn't support different input and output types, and adds a test for such a case (the test won't compile without that `constexpr` keyword).

Authors:
  - Louis Sugy (https://github.com/Nyrio)

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

URL: #296
Using the latest `cuco` commit hash as the `GIT_TAG` for release purposes.

Tested by successfully building RAFT using this change.

Authors:
  - Rick Ratzel (https://github.com/rlratzel)

Approvers:
  - Chuck Hastings (https://github.com/ChuckHastings)
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #304
Removes `-g` from the compile commands generated by distutils to compile Cython files.

This will make our container images, conda packages, and python wheels smaller.
@ajschmidt8
Copy link
Member Author

rerun tests

@ajschmidt8 ajschmidt8 merged commit 3b0a6d2 into main Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.