[GitHub] [tvm] apeskov commented on a diff in pull request #13849: [RUNTIME] Fix the manual determination of cores in FillDataForMeasure

["apeskov (via GitHub)" <gi...@apache.org>]

apeskov commented on code in PR #13849:
URL: https://github.com/apache/tvm/pull/13849#discussion_r1088719361


##########
src/runtime/contrib/random/mt_random_engine.cc:
##########
@@ -192,12 +192,12 @@ class RandomEngine {
     struct ParallelTask {
       static int RunTask(int task_id, TVMParallelGroupEnv* penv, void* cdata) {
         ParallelTask* task = static_cast<ParallelTask*>(cdata);
-        task->Run(task_id);
+        task->Run(task_id, penv->num_task);
         return 0;
       }
 
-      void Run(int i) {
-        int64_t chunk_size = size / num_threads;
+      void Run(int i, int num_threads) {
+        int64_t chunk_size = ceil(size / num_threads);

Review Comment:
   > is it still correct to use this number as a divider for size.
   
   Yes and no simultaneously. With current implementation it is incorrect. There is missed check at line below `int64_t st = std::min(i * chunk_size, size);`. With adding this line it will be correct.
   
   > how prev->num_task is correlate with number of threads?
   
   It's one and the same.  TVMBackendParallelLaunch API [reference](https://github.com/apache/tvm/blob/56771a87d1560f8963cf745ad093ffc3d83f3f6a/include/tvm/runtime/c_backend_api.h#L119-L146) 
   
   > Is it possible that size is less than num_threads?
   
   Yes, it's possible.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org