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/07 22:49:59 UTC
[GitHub] [incubator-tvm] tqchen opened a new issue #4648: [RFC] Unify IR
Node and Reference Class Naming Convention
tqchen opened a new issue #4648: [RFC] Unify IR Node and Reference Class Naming Convention
URL: https://github.com/apache/incubator-tvm/issues/4648
In many node objects we introduced recently, we use the Node suffix to hold IR/AST nodes and no suffix for reference classes.
```c++
class RangeNode : public Object {
public:
Expr min;
Expr extent;
};
class Range : public ObjectRef {
public:
// constructor
Range(Expr begin, Expr end);
// static factory.
static Range make_by_min_extent(Expr min, extent);
};
```
In the above example:
- ```RangeNode``` is the container object.
- ```Range``` is the reference class.
- New objects are constructed via constructors in the reference class.
- We also define clear factory function ```make_by_min_extent`` in the reference class.
However, due to legacy reasons, we also have a few different styles in the codebase.
In particular, some IR container object does have a ```Node``` suffix or a reference class.
Because not every IR container object has a reference class,
the references are constructed via static make functions in the object container class
instead. The code block below shows an example:
```
class IntImm : public ExprNode {
public:
Expr make(DataType dtype, int64_t value);
};
```
## Proposal
This RFC proposes to adopt a single style throughout the codebase.
We propose to adopt the following convention as a strawman:
- Always use ```Node``` as the suffix for IR node container objects.
- Always introduce a reference class(which does not have suffix) to each container object.
- Because we have a reference class for each object class, we can use a constructor to directly
construct a reference class
- In some instances where we need a clearly named factory function,
we can put them as a static function of the corresponding reference class.
Notably, for object containers that are not part of IR, some end in Obj(e.g. ADTObj) and some end in Node(e.g. ModuleNode).
This RFC will not seek to consolidate these names, but it would also be great to hear from everyone's thoughts about this topic as well.
The main advantage of the proposed style is that the C++ code will be more consistent with the python version.
It is also more natural to construct reference via constructors.
Migrated from https://discuss.tvm.ai/t/unify-naming-convetion-for-ir-node-and-reference-classes
----------------------------------------------------------------
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 #4648: [RFC] Unify IR
Node and Reference Class Naming Convention
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4648: [RFC] Unify IR Node and Reference Class Naming Convention
URL: https://github.com/apache/incubator-tvm/issues/4648#issuecomment-591197271
Update, the TIR namespace is already updated. but we have yet updated all nodes in the relay namespace
----------------------------------------------------------------
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 #4648: [RFC] Unify
IR Node and Reference Class Naming Convention
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #4648: [RFC] Unify IR Node and Reference Class Naming Convention
URL: https://github.com/apache/incubator-tvm/issues/4648#issuecomment-576818889
NOTE: as of now, we have updated the names in the ir namespace to conform to the standard.
We still need to update names in the
- [x] relay namespace
- [x] tir namespace once we start to add new nodes
@jroesch @slyubomirsky @MarisaKirisame please let me know if you are interested in help doing the relay side of changes
----------------------------------------------------------------
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 #4648: [RFC] Unify
IR Node and Reference Class Naming Convention
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #4648: [RFC] Unify IR Node and Reference Class Naming Convention
URL: https://github.com/apache/incubator-tvm/issues/4648#issuecomment-576818889
NOTE: as of now, we have updated the names in the ir namespace to conform to the standard.
We still need to update names in the
- [ ] relay namespace
- [ ] tir namespace once we start to add new nodes
@jroesch @slyubomirsky @MarisaKirisame please let me know if you are interested in help doing the relay side of changes
----------------------------------------------------------------
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 closed issue #4648: [RFC] Unify IR Node and
Reference Class Naming Convention
Posted by GitBox <gi...@apache.org>.
tqchen closed issue #4648: [RFC] Unify IR Node and Reference Class Naming Convention
URL: https://github.com/apache/incubator-tvm/issues/4648
----------------------------------------------------------------
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] ANSHUMAN87 commented on issue #4648: [RFC] Unify IR
Node and Reference Class Naming Convention
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on issue #4648: [RFC] Unify IR Node and Reference Class Naming Convention
URL: https://github.com/apache/incubator-tvm/issues/4648#issuecomment-594316981
> Update, the TIR namespace is already updated. but we have yet updated all nodes in the relay namespace
@tqchen : I was checking for relay part as you mentioned here. But i found all the places it is taken care as per https://github.com/apache/incubator-tvm/issues/4647. I like to help in this case. But i cant find the area which you are referring here. If i am mistaken, would you please point me in the right direction. 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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #4648: [RFC] Unify IR
Node and Reference Class Naming Convention
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4648: [RFC] Unify IR Node and Reference Class Naming Convention
URL: https://github.com/apache/incubator-tvm/issues/4648#issuecomment-576818889
NOTE: as of now, we have updated the names in the ir namespace to conform to the standard.
We still need to update names in the
- [ ] relay namespace
- [ ] tir namespace once we start to add new nodes
@jroesch @slyubomirsky @MarisaKirisame please let me know if you are interested in help doing that
----------------------------------------------------------------
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 #4648: [RFC] Unify IR
Node and Reference Class Naming Convention
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4648: [RFC] Unify IR Node and Reference Class Naming Convention
URL: https://github.com/apache/incubator-tvm/issues/4648#issuecomment-606253206
Close for now as most relay and tir nodes has been updated
----------------------------------------------------------------
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 #4648: [RFC] Unify
IR Node and Reference Class Naming Convention
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #4648: [RFC] Unify IR Node and Reference Class Naming Convention
URL: https://github.com/apache/incubator-tvm/issues/4648#issuecomment-576818889
NOTE: as of now, we have updated the names in the ir namespace to conform to the standard.
We still need to update names in the
- [ ] relay namespace
- [x] tir namespace once we start to add new nodes
@jroesch @slyubomirsky @MarisaKirisame please let me know if you are interested in help doing the relay side of changes
----------------------------------------------------------------
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