-
Notifications
You must be signed in to change notification settings - Fork 16
Non-default streams for filling matrix #32
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
Conversation
Commenting build and test reports here.
|
@@ -34,8 +34,7 @@ namespace adaboost | |||
*/ | |||
|
|||
template <class data_type_matrix> | |||
void fill(const data_type_matrix value, const MatrixGPU<data_type_matrix>&mat, unsigned block_size_x, unsigned block_size_y); |
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.
Why is this deleted?
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.
Since in current function, we are only taking one parameter, it number of streams. Instead of taking block_size_x and block_size_y. So the API is different now.
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.
Please do not deprecate APIs until utmost necessary. Please keep both APIs, we will use whatever required while implementing the AdaBoost algorithm.
Just copy the original fill
and related tests function from master
branch and add it to the right places to avoid conflicts. Do not modify the changes in your patch, just pick the right code from master
and paste it in your branch.
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.
Done, please have a look
Please doc strings as well. See the existing code for documentation style and similar docs for new functions. |
Co-authored-by: Gagandeep Singh <[email protected]>
@czgdp1807, is this ready to merge? |
[==========] Running 3 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 3 tests from Cuda
[ RUN ] Cuda.VectorGPU
[ OK ] Cuda.VectorGPU (50 ms)
[ RUN ] Cuda.MatrixGPU
[ OK ] Cuda.MatrixGPU (3323 ms)
[ RUN ] Cuda.MatricesGPU
[ OK ] Cuda.MatricesGPU (766 ms)
[----------] 3 tests from Cuda (4139 ms total)
[----------] Global test environment tear-down
[==========] 3 tests from 1 test suite ran. (4139 ms total)
[ PASSED ] 3 tests.
[==========] Running 4 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 4 tests from Core
[ RUN ] Core.Vector
[ OK ] Core.Vector (1 ms)
[ RUN ] Core.Matrices
[ OK ] Core.Matrices (0 ms)
[ RUN ] Core.Sum
[ OK ] Core.Sum (0 ms)
[ RUN ] Core.Argmax
[ OK ] Core.Argmax (0 ms)
[----------] 4 tests from Core (1 ms total)
[----------] Global test environment tear-down
[==========] 4 tests from 1 test suite ran. (1 ms total)
[ PASSED ] 4 tests. |
Please use https://github.com/codezonediitj/utils/blob/master/create_template.py for creating template instantiations for function prototypes automatically. |
References to other Issues or PRs or Relevant literature
Fixes #2
Brief description of what is fixed or changed
Implementing filling matrix in
adaboost::cuda::core
by using non default streams. Number of streams is passed as a parameter to the function, and each row of matrix gets filled by one of the streams. The stream to fill is chosen in a round robin fashion.Other comments
Initial code of filling using
n
streams was by @fiza11. @Tanvi141 worked on integrating that code into this code base as well as implementing round robin.