-
Notifications
You must be signed in to change notification settings - Fork 217
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
Cuda draft #71
Cuda draft #71
Conversation
pointers to it. | ||
*/ | ||
class Context { | ||
|
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.
@csukuangfj I wonder if I could get your help/advice on this...
Bear in mind that this CUDA stuff is not just CUDA, it is a proposal for a refactoring of the entire thing.
This "Context" class is my proposal for how we interact with external toolkits like PyTorch, but there
are some details that I'm not sure about how to do.
We'd then do something like the following (in a separate file where we #include the appropriate torch stuff).
class PytorchContext: public Context {
// ...
private:
Device device; // The device, e.g. CPU or a specific GPU.
Allocator *allocator; // the PyTorch allocator corresponding to this device.
std::unique_ptr<at::Tensor tensor_> tensor_; // tensor that holds the memory; this is a special case,
// where this Context is attached to a PyTorch tensor.
// If Duplicate() is called on a Context with a non-NULL
// tensor_ pointer we have to create
// a new Context that has a null one. Also, Deallocate() is
// a no-op in this case (memory will be freed, if appropriate,
// when the Context is deallocated and we let go of the unique_ptr).
};
The things that I'm a little unclear on right now are:
(a) We need a way to do the python binding. At some point we will want to turn a PyTorch array into some kind of Array1 with a Context attached that is really a PytorchContext. From that point we can pass it into other k2 things. I guess we could have some kind of TorchToK2() call that turns a Torch 1-d tensor into a K2 1-d tensor?
(b) How do we set up the build system? Bear in mind that it would be nice to make the torch stuff separable from the k2 stuff. I.e. we'd like to be able to work with other toolkits in future.
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.
I don't have much experience in CUDA.
To make the torch stuff separable from the k2 stuff, we should avoid things like at::Tensor
in k2 since it pulls source code dependencies from PyTorch.
@@ -0,0 +1,84 @@ | |||
#include "ragged.h" |
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.
@danpovey it seems you forget to push ragged.h
?
Don't worry, anything using at::Tensor can be put in a separate source file
and compiled optionally.
It's only accessed through a virtual class.
…On Sat, Jul 25, 2020 at 3:37 PM Fangjun Kuang ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In k2/csrc/cuda/context.h
<https://github.com/danpovey/k2/pull/71#discussion_r460377615>:
> +};
+
+
+/**
+ class Context is the main surface of interaction with external tensor libraries like
+ PyTorch; it allows us to use their notions of device and of memory allocation.
+ (in the case of PyTorch it would probably contain a Device and an Allocator).
+
+ We will sub-class this in several ways: with versions that wrap external toolkits
+ like PyTorch, and also a "native" version that's mostly for testing purposes.
+
+ This object should be allocated with std::shared_ptr, as that's how we store
+ pointers to it.
+*/
+class Context {
+
@danpovey <https://github.com/danpovey>
I don't have much experience in CUDA.
To make the torch stuff separable from the k2 stuff, we should avoid
things like at::Tensor in k2 since it pulls source code dependencies from
PyTorch.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/danpovey/k2/pull/71#discussion_r460377615>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZFLO4575YQIYC3EATBCIDR5KDSDANCNFSM4PGKYLRA>
.
|
* 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.
* more progress... * various progress... * Starting to simplify Ragged code * Make ragged arrays generic. * Progress... * Fix duplication
* Fix syntax error of ops_test.cu * Add a const version of k2::Array1::Context to fix quantifier error of array.h
* implement tensor * fix issues * implement tensor * revert to use array to implement dims and strides in tensor * add tests for tensor
* implement tensor * fix issues * implement tensor * revert to use array to implement dims and strides in tensor * add tests for tensor * add StorageSize() in tensor
* Various progress.. * Further implementation progress... * Further progress... * Various progress (in the middle of something... * Partial work...
* 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]>
* Various progress.. * Further implementation progress... * Further progress... * Various progress (in the middle of something... * Partial work... * Progress with ragged implementation... * CUDA streams
making PR for easier commenting