Refactor Unit Tests for OpenCL Kernels #3601
Conversation
djeong20
requested review from
DonghakPark,
EunjuYang,
SeoHyungjun,
again4you,
anyj0527,
baek2sm,
dkjung,
gichan-jang,
haehun,
jaeyun-jung,
jihochu,
jijoongmoon,
leemgs,
lhs8928,
myungjoo,
skykongkong8,
songgot and
wooksong
as code owners
This pull request refactors the unit tests for OpenCL kernels. The changes involve splitting the test files into three categories based on functionality. One for INT4-related tests, another for QK_K (GGML format) tests, and a third for the remaining BLAS tests. Additionally, it consolidates common helper functions used across the test files into separate utility header files. **Self-evaluation:** 1. Build test: [X]Passed [ ]Failed [ ]Skipped 2. Run test: [X]Passed [ ]Failed [ ]Skipped Signed-off-by: Donghyeon Jeong <[email protected]>
djeong20
force-pushed
the
refactor/opencl
branch
from
182812d to
27fbb41
Compare
| nntrainer::IniWrapper ini; | ||
| }; | ||
|
|
||
| #define GEN_TEST_INPUT_NHWC(input, eqation_i_j_k_l) \ |
There was a problem hiding this comment.
How about keeping the whitespace at the end of each line? And remove file diffs?
There was a problem hiding this comment.
I believe the previous version has incorrect formatting, likely due to being formatted with an outdated version.
(The last time this file was edited was a year ago!)
| // Initialize Activation | ||
| std::vector<float> activation = generate_random_vector<float, false>(M * K); | ||
| float *activations_f32_ptr = (float *)allocateSVM(M * K * sizeof(float)); | ||
| blas_cc->command_queue_inst_.enqueueSVMMap(activations_f32_ptr, |
There was a problem hiding this comment.
After use, Right?
blas_cc->command_queue_inst_.enqueueSVMUnmap(activations_f32_ptr, 0, nullptr, nullptr);
There was a problem hiding this comment.
Yes, you're right. However, I believe that unmap should be called before kernel execution and should be handled here.
| // weight 0 (3072 x 3072) | ||
| void *ws0 = allocateSVM(data_size_n0 / scale_group_size); | ||
| void *wq0 = allocateSVM(data_size_n0 / 2); | ||
| blas_cc->command_queue_inst_.enqueueSVMMap(wq0, data_size_n0 / 2, false); |
There was a problem hiding this comment.
Isn't 'true' better than 'false'?
There was a problem hiding this comment.
Great refactor! Splitting up that massive unittest_nntrainer_cl.cpp file was definitely needed. It makes the tests so much easier to navigate and maintain.
I really like how you've extracted the common helpers like allocateSVM and the random generators into nntrainer_test_util.h. That's a great example of DRY (Don't Repeat Yourself).
The file split logic (BLAS, INT4, QK_K) makes perfect sense.
Everything looks clean and well-organized. Thank you for improving the codebase structure! LGTM! ✨
3 participants
This pull request refactors the unit tests for OpenCL kernels.
The changes involve splitting the test files into three categories based on functionality.
One for INT4-related tests, another for QK_K (GGML format) tests, and a third for the remaining BLAS tests.
Additionally, it consolidates common helper functions used across the test files into separate utility header files.
Self-evaluation: