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/06/09 16:01:53 UTC
[GitHub] [incubator-tvm] ANSHUMAN87 opened a new pull request #5748: [REFACTOR][Relay] Migrate Id ObjectRef to not-null
ANSHUMAN87 opened a new pull request #5748:
URL: https://github.com/apache/incubator-tvm/pull/5748
Refer #5318
@tqchen, @jroesch, @zhiics, @junrushao1994 : Please help review, Thanks!
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5748: [REFACTOR][Relay] Migrate Id ObjectRef to not-null
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5748:
URL: https://github.com/apache/incubator-tvm/pull/5748#discussion_r436809003
##########
File path: include/tvm/relay/expr.h
##########
@@ -152,6 +152,9 @@ class Var;
/*! \brief Container for Var */
class VarNode : public ExprNode {
public:
+ VarNode(Id vid, Type type_annotation) : vid(vid), type_annotation(type_annotation) {}
+ VarNode() : vid("") {}
Review comment:
The not-null that involves composite objects are more complicated and this is not necessarily the best approach, let us wait a bit before we find a better solution
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] tqchen closed pull request #5748: [REFACTOR][Relay] Migrate Id ObjectRef to not-null
Posted by GitBox <gi...@apache.org>.
tqchen closed pull request #5748:
URL: https://github.com/apache/incubator-tvm/pull/5748
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] tqchen commented on pull request #5748: [REFACTOR][Relay] Migrate Id ObjectRef to not-null
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5748:
URL: https://github.com/apache/incubator-tvm/pull/5748#issuecomment-641526616
@junrushao1994 good pt, can you send another PR?
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] junrushao1994 commented on pull request #5748: [REFACTOR][Relay] Migrate Id ObjectRef to not-null
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #5748:
URL: https://github.com/apache/incubator-tvm/pull/5748#issuecomment-642203863
Do we have conclusions yet on what the best way is to support not-null with composite objects?
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #5748: [REFACTOR][Relay] Migrate Id ObjectRef to not-null
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #5748:
URL: https://github.com/apache/incubator-tvm/pull/5748#discussion_r436811810
##########
File path: include/tvm/relay/expr.h
##########
@@ -152,6 +152,9 @@ class Var;
/*! \brief Container for Var */
class VarNode : public ExprNode {
public:
+ VarNode(Id vid, Type type_annotation) : vid(vid), type_annotation(type_annotation) {}
+ VarNode() : vid("") {}
Review comment:
> The not-null that involves composite objects are more complicated and this is not necessarily the best approach, let us wait a bit before we find a better solution
@tqchen : Thanks! I also felt that this is not best way. We might have to change Node registration framework i believe. Will wait for your future guidance!
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] tqchen commented on pull request #5748: [REFACTOR][Relay] Migrate Id ObjectRef to not-null
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5748:
URL: https://github.com/apache/incubator-tvm/pull/5748#issuecomment-642226324
@junrushao1994 I will try to send a proposal around UnsafeNull
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] junrushao1994 commented on pull request #5748: [REFACTOR][Relay] Migrate Id ObjectRef to not-null
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #5748:
URL: https://github.com/apache/incubator-tvm/pull/5748#issuecomment-641501952
@tqchen Seems that some changes in my WIP PR have affected this PR https://ci.tvm.ai/blue/organizations/jenkins/tvm/detail/PR-5748/1/pipeline. We probably should "make clean" before build wasm
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] ANSHUMAN87 commented on pull request #5748: [REFACTOR][Relay] Migrate Id ObjectRef to not-null
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #5748:
URL: https://github.com/apache/incubator-tvm/pull/5748#issuecomment-640713504
This would be a baseline reference for other Relay nodes.
I have one query here. As you see i have to add 1 empty/default constructor here to resolve compile error i faced in [TVM_REGISTER_NODE_TYPE(VarNode)](https://github.com/apache/incubator-tvm/blob/6ae439c8c58dd0118d2f2c5d1c4bcb650df47104/src/relay/ir/expr.cc#L91).
Please suggest the best way to cover this! Thanks!
----------------------------------------------------------------
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