You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "LeshengJin (via GitHub)" <gi...@apache.org> on 2023/09/26 21:30:44 UTC

[GitHub] [tvm] LeshengJin opened a new pull request, #15826: [Disco] Advanced sharding support

LeshengJin opened a new pull request, #15826:
URL: https://github.com/apache/tvm/pull/15826

   (no comment)


-- 
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


[GitHub] [tvm] jinhongyii commented on a diff in pull request #15826: [Disco] Loading-time sharding support

Posted by "jinhongyii (via GitHub)" <gi...@apache.org>.
jinhongyii commented on code in PR #15826:
URL: https://github.com/apache/tvm/pull/15826#discussion_r1339039014


##########
src/runtime/disco/loader.cc:
##########
@@ -60,6 +61,10 @@ class ShardLoaderObj : public Object {
     const FileRecord* file;
     const ParamRecord* param;
     int shard_dim;
+    PackedFunc f_reorder_fp16_;

Review Comment:
   I'm thinking of future cases where we might need more kinds of preprocessing other than reorder. If we let users pass a preprocess function's base name in shard info, like `reorder_qkv`, and look for corresponding dtype variant `reorder_qkv_fp16` in IRModule automatically, it might be more flexible. If we want to do that, we can look up preprocess func when we call `Shard` instead of `Create`



-- 
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


[GitHub] [tvm] jinhongyii commented on a diff in pull request #15826: [Disco] Loading-time sharding support

Posted by "jinhongyii (via GitHub)" <gi...@apache.org>.
jinhongyii commented on code in PR #15826:
URL: https://github.com/apache/tvm/pull/15826#discussion_r1339018616


##########
src/runtime/disco/loader.cc:
##########
@@ -60,6 +61,10 @@ class ShardLoaderObj : public Object {
     const FileRecord* file;
     const ParamRecord* param;
     int shard_dim;
+    PackedFunc f_reorder_fp16_;

Review Comment:
   Why do we need 3 different functions for different dtypes? Why can't we pass different packed func in shard info when dtypes are differnt?



-- 
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


[GitHub] [tvm] jinhongyii commented on a diff in pull request #15826: [Disco] Loading-time sharding support

Posted by "jinhongyii (via GitHub)" <gi...@apache.org>.
jinhongyii commented on code in PR #15826:
URL: https://github.com/apache/tvm/pull/15826#discussion_r1339018616


##########
src/runtime/disco/loader.cc:
##########
@@ -60,6 +61,10 @@ class ShardLoaderObj : public Object {
     const FileRecord* file;
     const ParamRecord* param;
     int shard_dim;
+    PackedFunc f_reorder_fp16_;

Review Comment:
   Why do we need 3 different functions for different dtypes? Why can't we pass different packed func in shard info when dtypes are differnt?



##########
src/runtime/disco/loader.cc:
##########
@@ -60,6 +61,10 @@ class ShardLoaderObj : public Object {
     const FileRecord* file;
     const ParamRecord* param;
     int shard_dim;
+    PackedFunc f_reorder_fp16_;

Review Comment:
   Can we rename to f_preprocess, because it's more general?



-- 
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


[GitHub] [tvm] junrushao commented on pull request #15826: [Disco] Loading-time sharding support

Posted by "junrushao (via GitHub)" <gi...@apache.org>.
junrushao commented on PR #15826:
URL: https://github.com/apache/tvm/pull/15826#issuecomment-1736533921

   CC: @jinhongyii 


-- 
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


[GitHub] [tvm] jinhongyii commented on pull request #15826: [Disco] Loading-time sharding support

Posted by "jinhongyii (via GitHub)" <gi...@apache.org>.
jinhongyii commented on PR #15826:
URL: https://github.com/apache/tvm/pull/15826#issuecomment-1741877593

   Can we separate reorganizing function and sharding function again? I'm thinking of some situation where there can be 2 reorganizing function ( doing nothing and combine qkv), and n different sharding functions( different tensor shape and different sharding dimension). In this case, we will need at most 2n functions in total in IRModule. 
   


-- 
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


Re: [PR] [Disco] Loading-time sharding support [tvm]

Posted by "junrushao (via GitHub)" <gi...@apache.org>.
junrushao merged PR #15826:
URL: https://github.com/apache/tvm/pull/15826


-- 
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


[GitHub] [tvm] jinhongyii commented on a diff in pull request #15826: [Disco] Loading-time sharding support

Posted by "jinhongyii (via GitHub)" <gi...@apache.org>.
jinhongyii commented on code in PR #15826:
URL: https://github.com/apache/tvm/pull/15826#discussion_r1339039014


##########
src/runtime/disco/loader.cc:
##########
@@ -60,6 +61,10 @@ class ShardLoaderObj : public Object {
     const FileRecord* file;
     const ParamRecord* param;
     int shard_dim;
+    PackedFunc f_reorder_fp16_;

Review Comment:
   I'm thinking of future cases where we might need more kinds of preprocessing other than reorder. If we let users pass a preprocess function's base name in shard info, like r`eorder_qkv`, and look for corresponding dtype variant `reorder_qkv_fp16` in IRModule automatically, it might be more flexible. If we want to do that, we can look up preprocess func when we call `Shard` instead of `Create`



-- 
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