You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/01/03 23:06:07 UTC

[GitHub] [incubator-tvm] tqchen opened a new pull request #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars

tqchen opened a new pull request #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars
URL: https://github.com/apache/incubator-tvm/pull/4615
 
 
   Currently, we use a tvm::Var to represent a placeholder for shapes in generic types.
   This is not necessary for GlobalTypeVar(as we never parameterize by shape var),
   and is a bit twisted for TypeVar.
   
   As we move to a unified type system, we want to break the dependency
   from the base TypeVar(which is shared across the languages) from the expression.
   Note that it is fine for TensorType to depend on Expr.
   
   One alternative solution to embed the Var would be to introduce a TypeVarExpr,
   which can wrap a TypeVar as Expr. However, this new alternative won't be
   natural until we migrate the type to the global scope.
   
   Lucikly, we have not yet start to depend on the shape parameterization heavily yet.
   
   This PR removes the tvm::Var from the typevars. We will follow up with another
   PR to migrate the types to a base location. After that, we should be able to
   use the more elegant approach via TypeVarExpr.
   
   Thanks for contributing to TVM!   Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on issue #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars

Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars
URL: https://github.com/apache/incubator-tvm/pull/4615#issuecomment-570751671
 
 
   @MarisaKirisame I think we could bring a separate discuss thread in the forum for the type system design. My previous takeaway with @jroesch was that we don't want to introduce a full dependent type.
   
   Currently the Expr used in the TensorType is not part of relay expression, and only works on integer arithmetics to expression shape relations.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen edited a comment on issue #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars

Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars
URL: https://github.com/apache/incubator-tvm/pull/4615#issuecomment-570724800
 
 
   cc @wweic @jroesch @yzhliu @MarisaKirisame @slyubomirsky 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on issue #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars

Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars
URL: https://github.com/apache/incubator-tvm/pull/4615#issuecomment-570724800
 
 
   cc @wweic @jroesch 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] MarisaKirisame edited a comment on issue #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars

Posted by GitBox <gi...@apache.org>.
MarisaKirisame edited a comment on issue #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars
URL: https://github.com/apache/incubator-tvm/pull/4615#issuecomment-570750060
 
 
   having type that use expr is called dependent type. typically, instead of typevarexpr, ppl unify the "type",  "kind" and "expr" data type into one single type. Why dont we merge type and expr? It will, e.g. make attribute not special anymore, become first class citizen so one can very easily define function in tvm that basically call another function. right now it is not possible unless you do it in C++, because you have to fill out typerelation and do dispatching yourself there.
   @jroesch 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen merged pull request #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars

Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars
URL: https://github.com/apache/incubator-tvm/pull/4615
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] MarisaKirisame commented on issue #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars

Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on issue #4615: [REFACTOR][TYPE] Remove un-necessary var sub-field in TypeVars
URL: https://github.com/apache/incubator-tvm/pull/4615#issuecomment-570750060
 
 
   having type that use expr is called dependent type. typically, instead of typevarexpr, ppl unify the "type",  "kind" and "expr" data type into one single type. Why dont we merge type and expr? It will, e.g. make attribute not special anymore, become first class citizen so one can very easily define function in tvm that basically call another function. right now it is not possible unless you do it in C++, because you have to fill out typerelation and do dispatching yourself there.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services