tvm-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tianqi Chen <notificati...@github.com>
Subject [dmlc/tvm] [RFC] Node Construction Style: Use Constructor When Possible (#3223)
Date Tue, 21 May 2019 21:43:13 GMT
Node system lays the foundation to build TVM's AST structures. Each AST object corresponds
to a subclass of Node. Besides the Node itself, we also usually defines a reference type which
can be used to store a strong reference to the node. The following code example shows how
to define Node ```XYZNode``` and reference type ```XYZ```.

```c++
class XYZ;

class XYZNode : public Node {
 public:
   static XYZ make(args);
};

class XYZ : public NodeRef {
  public:
   ...
};

void MyFunction() {
   XYZ ref = XYZNode::make(args);
}
```

Historically, we introduced a static factory function ```make``` to each Node type which creates
a new reference to the Node(as shown in the above code). This is less natural compared to
direct usage of a constructor or moves the static factory function into the reference class.
As shown in below.

```c++
class XYZNode : public Node {
 public:
};

class XYZ : public NodeRef {
  public:
   XYZ(args);
   ...
};

void MyFunction() {
   // use constructor.
   XYZ ref = XYZ(args);
}
```

At the moment, not all the Node have a specific reference type -- this is also the reason
why we adopt make in the Node historically. However, we start to see a trend of giving a specific
reference type to each of the new Node introduced(e.g. in relay). I would also like to mention
that we have already transitioned the python API to directly use the constructor-style code.
Favor constructors in c++ will make both sides of the code to be more consistent.

This RFC tries to see if we want to migrate to the new style, specifically:

- For Nodes that do not have a corresponding reference type, keep the make function on Node.
- For Nodes that have a reference type
   - Favor constructor over XYZNode::make
   - Favor static factory functions in the reference type.

Most of the changes will be quite mechanical and we can do it incrementally. Please share
your thoughts.





-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3223
Mime
  • Unnamed multipart/alternative (inline, 7-Bit, 0 bytes)
View raw message