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

Update the build system to support Cuda. #75

Merged
merged 4 commits into from
Jul 29, 2020

Conversation

csukuangfj
Copy link
Collaborator

@csukuangfj csukuangfj commented Jul 27, 2020

Now Cub is supported and the cuda branch is paritially compilable.

I've added a naive implementation for context and a test for ExclusivePrefixSum.

A google colab notebook is also added in case someone wants to try it out but does not have a GPU.


Note that CMake 3.8 is required. C++ 14 is used to remove the warnings reported by Cub.

@danpovey @qindazhu
please have a review.

k2/csrc/cuda/utils_inl.h Outdated Show resolved Hide resolved
@@ -10,9 +10,11 @@ on:
push:
branches:
- master
- cuda_draft
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we should create a new branch (e.g. cuda), as Dan may push to cuda_draft directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've disabled the GitHub workflow for the CUDA branch since it requires CUDA and won't compile, which always fails.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Once this pullrequest is merged, we can create a new branch from cuda_draft and leave cuda_draft for Dan.

k2.ipynb Outdated Show resolved Hide resolved
k2/csrc/cuda/context.h Outdated Show resolved Hide resolved
k2/csrc/cuda/context.cu Outdated Show resolved Hide resolved
@qindazhu
Copy link
Collaborator

Fantastic work! Only a few comments, +2!

@csukuangfj
Copy link
Collaborator Author

thanks, I will resolve the comments tonight.

@danpovey
Copy link
Collaborator

Great, thanks!

@danpovey danpovey merged commit fda3b83 into k2-fsa:cuda_draft Jul 29, 2020
@csukuangfj csukuangfj deleted the fangjun-cuda branch July 29, 2020 10:19
@danpovey
Copy link
Collaborator

@csukuangfj why did you upgrade the build to c++14 from c++11, and what file did you edit to do that?
It's causing me problems building it, as the version of gcc here doesn't support that. I don't like to use any newer
version of c++ than we need to.

@csukuangfj
Copy link
Collaborator Author

csukuangfj commented Jul 30, 2020

To use c++11, you can change
https://github.com/danpovey/k2/blob/a662f9bfea895ad5eef09abcb8a02b6657d7c1f8/CMakeLists.txt#L42

to

set(CMAKE_CXX_STANDARD 11 CACHE STRING "The C++ version to be used.") 

The reason to switch to c++14 is that the following code from CUB: https://github.com/NVlabs/cub/blob/master/cub/util_cpp_dialect.cuh#129

#if !defined(CUB_IGNORE_DEPRECATED_CPP_DIALECT) && CUB_CPP_DIALECT < 2014 && \
    (CUB_CPP_DIALECT != 2011 || !defined(CUB_IGNORE_DEPRECATED_CPP_11))
  CUB_COMPILER_DEPRECATION(C++14, pass -std=c++14 to your compiler);
#endif

issues warnings during the compilation if you are not using c++14.

@csukuangfj
Copy link
Collaborator Author

I can suppress the CUB warnings by deleting CUB_COMPILER_DEPRECATION(C++14, pass -std=c++14 to your compiler);
if you like.

@danpovey
Copy link
Collaborator

danpovey commented Jul 30, 2020 via email

@csukuangfj
Copy link
Collaborator Author

I will make a change this night.

You can use

cd build
cmake -DCMAKE_CXX_STANDARD=11 ..

to switch to c++11 without changing the CMakeLists.txt

@csukuangfj csukuangfj restored the fangjun-cuda branch July 30, 2020 08:06
@danpovey
Copy link
Collaborator

danpovey commented Jul 30, 2020 via email

danpovey added a commit that referenced this pull request Aug 21, 2020
* Some code drafts

* Updates to determinize draft..

* Progress on draft of CUDA stuff (sorry: lots of inconsistencies still)

* Rework context/memory management for easier integration with external toolkits

* Updates to Context

* Add ragged.h

* Add shape.h

* Various fixes, added compose.cc

* implement GetContext with parameter pack (#73)

* Various fixes etc.

* Update license; various updates on cuda stuff

* Add utils header

* Update the build system to support Cuda. (#75)

* update the build system to support cuda.

* add a google colab example for Cuda test.

* enable CI for the cuda_draft branch.

* resolve some comments.

* Updates to compose algo.

* Fix couple build issues

* move shape to ragged_shape

* More progress...

* More progress... won't compile

* More progress on CUDA draft (wont compile, sorry)

* Working on composition...

* matrix transpose in cuda (simple version) (#84)

* more progress...

* add performace test functions (#85)

* various progress...

* fix build issues (#87)

Co-authored-by: Haowen Qiu <[email protected]>
Co-authored-by: Fangjun Kuang <[email protected]>
danpovey added a commit that referenced this pull request Sep 1, 2020
* Cuda draft2 (#86)

* Some code drafts

* Updates to determinize draft..

* Progress on draft of CUDA stuff (sorry: lots of inconsistencies still)

* Rework context/memory management for easier integration with external toolkits

* Updates to Context

* Add ragged.h

* Add shape.h

* Various fixes, added compose.cc

* implement GetContext with parameter pack (#73)

* Various fixes etc.

* Update license; various updates on cuda stuff

* Add utils header

* Update the build system to support Cuda. (#75)

* update the build system to support cuda.

* add a google colab example for Cuda test.

* enable CI for the cuda_draft branch.

* resolve some comments.

* Updates to compose algo.

* Fix couple build issues

* move shape to ragged_shape

* More progress...

* More progress... won't compile

* More progress on CUDA draft (wont compile, sorry)

* Working on composition...

* matrix transpose in cuda (simple version) (#84)

* more progress...

* add performace test functions (#85)

* various progress...

* fix build issues (#87)

Co-authored-by: Haowen Qiu <[email protected]>
Co-authored-by: Fangjun Kuang <[email protected]>

* cmake: set "Debug" as default, and add "-O0" for debugging (#83)

* cmake: set "Debug" as default, and add "-O0" for debugging

* make cmakelists.txt more pratical

* change style of cmake custom variables

* [WIP] CUDA error checking/debugging (#94)

* [WIP] CUDA error checking/debugging

add debug.cuh and related changes.

* update #1 after review

* update

* update

* trivials

* replace get_filename_component, as it rises cmake version warning

* add `K2_ASSERT, K2_CHECK_EQ, K2_PARANOID_ASSERT` 

and update for last review; make some trival changes

* typo

* trivals

* fix a error

* typo

* Update debug.h

* make last change consist with its comment

* update to review

* update to review

* update as Dan required

- remove macro redefine guards
- replace K2_MAKE_ERROR_CHECK with NDEBUG

* changes make gtest stop warn about usage of *_DEATH

* remove utility macro about archtecture  and the related file.

* remove more complex less helpful cmake gtest module, and revert back

* fix pointer pos style

* continue to remove gtest util

* rm special variadic printf for clang 

as I tried with clang with cuda, and finally the previous error dismissed: 
"function not viable: requires 2 arguments, but 4 were provided                            
extern int vprintf (const char *__restrict __format, _G_va_list __arg);"

* K2_CUDA_SAFE_CALL

Never distinguish API or KERNEl call, instead merge into one: K2_CUDA_SAFE_CALL.

* fix to review

* update to review

- fix a twice error code checking issue
- change macro name `K2_CUDA_CHECK_ERROR` to `K2_CUDA_PRINT_ERROR` as its effect
- add a optional arg `abort` to control abort if hit error
- trivals
fix

* just revert name: `K2_CUDA_CHECK_ERROR`

* update to review (#103)

* update to review

- fix a twice error code checking issue
- change macro name `K2_CUDA_CHECK_ERROR` to `K2_CUDA_PRINT_ERROR` as its effect
- add a optional arg `abort` to control abort if hit error
- trivals
fix

* just revert name: `K2_CUDA_CHECK_ERROR`

* rm two files that @dan deleted on master

Co-authored-by: Daniel Povey <[email protected]>
Co-authored-by: Haowen Qiu <[email protected]>
Co-authored-by: Fangjun Kuang <[email protected]>
@csukuangfj csukuangfj mentioned this pull request Sep 9, 2020
@csukuangfj csukuangfj deleted the fangjun-cuda branch September 14, 2020 02:33
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.

3 participants