-
Notifications
You must be signed in to change notification settings - Fork 191
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] Migrating cuml comms to raft::comms #7
Merged
Merged
Changes from 49 commits
Commits
Show all changes
51 commits
Select commit
Hold shift + click to select a range
fd38dbb
Initial commit of raft comms
cjnolet 8a6e054
Removing cuml verbiage
cjnolet 2bfc0dc
Renaming to comms_t, removing logs for now, adding to handle
cjnolet f129710
Looking like it's building!
cjnolet a0c13af
Adding NCCL and UCX build for tests
cjnolet 7ad2053
Merge branch 'fea-ext-migrate-cumlHandle_impl-into-raft' into fea-ext…
cjnolet cb19482
Consolidating injection functions
cjnolet 1d66637
Merge branch 'fea-ext-migrate-cumlHandle_impl-into-raft' into fea-ext…
cjnolet 1548177
Making progress on Python. Need to refactor the comms interface a li…
cjnolet 5d37e9e
Cython is building!
cjnolet bd1b876
The comms tests pass!!!
cjnolet 98496d3
Fixing flake8 style
cjnolet fb52f79
Running clang format and removing getDataType
cjnolet d666314
Merge remote-tracking branch 'github/branch-0.14' into fea-ext-comms
cjnolet 46a27eb
Cleaning up
cjnolet fc6ba2e
Fixing python style
cjnolet 842f533
Fixing cpp style
cjnolet 43f0d69
Adding copyright headers
cjnolet ac6e699
Adding init py for tests
cjnolet c442d87
Adding license headers and consistent namespacing
cjnolet 32e63c1
More cleanup
cjnolet d377347
Cleaning up raft.dask.common.Comms
cjnolet e1b4ea7
Ignoring raft egg artifacts
cjnolet 736696b
Cleaning up raft dask utils
cjnolet 55d9dfd
More cleanup, copyright headers, and docs
cjnolet 6eb0325
Removing the last of references to cuml
cjnolet b65e20d
Fixing python style
cjnolet 7a845cb
Fixing c++ style
cjnolet c269300
Updating changelog
cjnolet d4aa5c5
Testing non-ucx cluster for pytests
cjnolet 1ee1363
Implementing review feedback
cjnolet c165a36
More review feedback
cjnolet 12f3db7
Fixing style
cjnolet aae4625
FIX Use relative imports
dantegd bc39321
Adding compile-time templates for comms_t to make interaction more st…
cjnolet e9c0995
Merge pull request #1 from dantegd/fea-ext-comms-pr1
cjnolet 3150fbd
Using std::this_thread::yield instead of pthread_yield()
cjnolet 5628ad2
Adding python tests for collective functions
cjnolet 488d0d5
Running cpp style
cjnolet 3d362d0
Updating tabbing for pytests
cjnolet 417a4bd
Following clang tidy standards
cjnolet cb92349
Moving get_type out of comms_t
cjnolet 6e9025d
More review feedback
cjnolet 28f8101
Running cpp style check
cjnolet 1a18553
Nccl red op
cjnolet a22dd62
Raising an exception to get around gcc issue
cjnolet fbd12aa
Using static for functions for now
cjnolet cc71ccb
Fixing style
cjnolet dc123b2
Fixing more relative imports
cjnolet 60597f3
final updates based on feedback
cjnolet 5bbb8be
Merge branch 'branch-0.15' into fea-ext-comms
afender File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ log | |
.ipynb_checkpoints | ||
.DS_Store | ||
dask-worker-space/ | ||
*.egg-info/ | ||
## eclipse | ||
.project | ||
.cproject | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
# | ||
# Copyright (c) 2019-2020, NVIDIA CORPORATION. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
|
||
cmake_minimum_required(VERSION 3.14 FATAL_ERROR) | ||
project(comms LANGUAGES CXX CUDA) | ||
|
||
set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_SOURCE_DIR}/cmake") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need this module-path set command here, in a module? |
||
|
||
if(NOT NCCL_PATH) | ||
find_package(NCCL REQUIRED) | ||
else() | ||
message("-- Manually set NCCL PATH to ${NCCL_PATH}") | ||
set(NCCL_INCLUDE_DIRS ${NCCL_PATH}/include) | ||
set(NCCL_LIBRARIES ${NCCL_PATH}/lib/libnccl.so) | ||
endif(NOT NCCL_PATH) | ||
|
||
set(CMAKE_CXX_STANDARD 14) | ||
set(CMAKE_CXX_STANDARD_REQUIRED ON) | ||
|
||
find_package(UCX) | ||
include_directories(${UCX_INCLUDE_DIRS}) | ||
|
||
include_directories( ${NCCL_INCLUDE_DIRS} ) | ||
list(APPEND RAFT_LINK_LIBRARIES ${NCCL_LIBRARIES}) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is probably a remnant of a debug session?