[REFACTOR] Streamline Function Attr interface. #5045

tqchen · 2020-03-11T21:36:22Z

There has been quite a few recent changes that depends heavily on
the function attr interface. This PR streamlines that interface by introducing
two APIs that covers most of the usages.

Thanks to @jroesch for discussing the API naming convention.

GetAttr which gets a typed object for a given key
- HasNonzeroAttr is a quick helper that calls GetAttr to quickly check an attribute
WithAttr that creates a new function object with the given attr
- The API comes with copy on write optimization to avoid multiple copies
- We deliberately pick the prefix With(instead of Set) to indicate this
  function does not mutate the original input.

On the python side:

We allow read access via func.attrs (which is a DictAttr)
e.g. func.attrs["__param__"]
func.with_attrs to create a new instance with updated attrs.

We also get rid of the small wrapper functions and make sure the API centered around
the GetAttr and HasNonzeroAttr interface.

This PR also changes the function construction to follow the new convention.

cc @jroesch @zhiics @mbaret

tqchen · 2020-03-11T21:38:57Z

NOTE: I have not removed the UseDefaultCompiler member function, however, we should remove that in a followup PR by simply setting the default compiler to None and we could use GetAttr<StringImm>(attr::kCompiler).defined() to check whether the customization is defined.

StringImm should be changed to String once #4628 is merged

cc @comaniac @zhiics

zhiics

LGTM. Only a few nitpicks.

zhiics · 2020-03-11T21:47:50Z

include/tvm/ir/attrs.h

+class DictAttrs : public Attrs {
+ public:
+  /*!
+   * \brief Consruct a Attrs backed by DictAttrsNode.


s/Consruct/Construct

zhiics · 2020-03-11T21:48:26Z

include/tvm/ir/function.h

+ * \brief Base node of all functions.
+ *
+ * We support several variants of functions throughout the stack.
+ * All of the functions shares the same type system(via checked_type)


s/shares/share

zhiics · 2020-03-12T00:25:57Z

include/tvm/ir/function.h

+   * \code
+   *
+   *  void HasNonzeroAttrExample(const BaseFunc& f) {
+   *    if (f->HasNonzeroAttr(attr::Inline)) {


zhiics · 2020-03-12T00:26:55Z

include/tvm/relay/function.h

+ * \returns The new function with updated attributes.
+ *
+ * \note This function performs copy on write optimization for func.
+ *       If we move an uniquely referenced func into WithAttr,


tqchen · 2020-03-12T01:48:51Z

thanks @zhiics please take another look

siju-samuel · 2020-03-12T02:28:58Z

include/tvm/ir/function.h

+ * \file tvm/ir/expr.h
+ * \brief Base expr nodes in TVM.


Update file & brief here

siju-samuel · 2020-03-12T02:29:26Z

src/ir/function.cc

+ */
+
+/*!
+ * \file src/tvm/ir/function.cc


src/ir/function.cc

siju-samuel · 2020-03-12T02:29:47Z

src/relay/ir/function.cc

+ */
+
+/*!
+ * \file src/tvm/relay/ir/function.cc


same as above

zhiics

@tqchen LGTM, please fix @siju-samuel's comments.

There has been quite a few recent changes that depends heavily on the function attr interface. This PR streamlines that interface by introducing two APIs that covers most of the usages. - GetAttr which gets a typed object for a given key - HasNonzeroAttr is a quick helper that calls GetAttr to quickly check an attribute - WithAttr that creates a new function object with the given attr - The API comes with copy on write optimization to avoid multiple copies - We deliberately pick the prefix With(instead of Set) to indicate this function does not mutate the original input. On the python side: - We allow read access via func.attrs (which is a DictAttr) - func.with_attrs to create a new instance with updated attrs. We also get rid of the small wrapper functions and make sure the API centered around the GetAttr and HasNonzeroAttr interface. This PR also changes the function construction to follow the new convention.

siju-samuel

LGTM

zhiics · 2020-03-12T15:34:42Z

Thanks @tqchen, @ZihengJiang, @siju-samuel

* [REFACTOR] Streamline Function Attr interface. There has been quite a few recent changes that depends heavily on the function attr interface. This PR streamlines that interface by introducing two APIs that covers most of the usages. - GetAttr which gets a typed object for a given key - HasNonzeroAttr is a quick helper that calls GetAttr to quickly check an attribute - WithAttr that creates a new function object with the given attr - The API comes with copy on write optimization to avoid multiple copies - We deliberately pick the prefix With(instead of Set) to indicate this function does not mutate the original input. On the python side: - We allow read access via func.attrs (which is a DictAttr) - func.with_attrs to create a new instance with updated attrs. We also get rid of the small wrapper functions and make sure the API centered around the GetAttr and HasNonzeroAttr interface. This PR also changes the function construction to follow the new convention. * Address review comments * Address review comments * Fix doxygen path

tqchen force-pushed the attrs branch from 8865437 to 45ab686 Compare March 11, 2020 21:39

zhiics reviewed Mar 12, 2020

View reviewed changes

siju-samuel reviewed Mar 12, 2020

View reviewed changes

zhiics approved these changes Mar 12, 2020

View reviewed changes

tqchen added 3 commits March 11, 2020 20:58

Address review comments

6626227

Address review comments

e5b84df

tqchen force-pushed the attrs branch from 914d1f1 to e5b84df Compare March 12, 2020 04:01

Fix doxygen path

4d7fcbc

siju-samuel approved these changes Mar 12, 2020

View reviewed changes

ZihengJiang approved these changes Mar 12, 2020

View reviewed changes

zhiics merged commit ec86d7f into apache:master Mar 12, 2020

zhiics added the status: accepted label Mar 12, 2020

tqchen deleted the attrs branch March 16, 2020 16:21

slyubomirsky mentioned this pull request Mar 17, 2020

[API change] Include tvm/relay/function.h uwsampl/relay-aot#38

Merged

zhiics mentioned this pull request Mar 17, 2020

[Realy] Replace UseDefaultCompiler with GetAttr #5088

Merged

ZihengJiang mentioned this pull request Sep 25, 2020

TVM v0.7 Release Note Candidate #6486

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REFACTOR] Streamline Function Attr interface. #5045

[REFACTOR] Streamline Function Attr interface. #5045

tqchen commented Mar 11, 2020 •

edited

Loading

tqchen commented Mar 11, 2020

zhiics left a comment

zhiics Mar 11, 2020

zhiics Mar 11, 2020

zhiics Mar 12, 2020

zhiics Mar 12, 2020

tqchen commented Mar 12, 2020

siju-samuel Mar 12, 2020

siju-samuel Mar 12, 2020

siju-samuel Mar 12, 2020

zhiics left a comment

siju-samuel left a comment

zhiics commented Mar 12, 2020

[REFACTOR] Streamline Function Attr interface. #5045

[REFACTOR] Streamline Function Attr interface. #5045

Conversation

tqchen commented Mar 11, 2020 • edited Loading

tqchen commented Mar 11, 2020

zhiics left a comment

Choose a reason for hiding this comment

zhiics Mar 11, 2020

Choose a reason for hiding this comment

zhiics Mar 11, 2020

Choose a reason for hiding this comment

zhiics Mar 12, 2020

Choose a reason for hiding this comment

zhiics Mar 12, 2020

Choose a reason for hiding this comment

tqchen commented Mar 12, 2020

siju-samuel Mar 12, 2020

Choose a reason for hiding this comment

siju-samuel Mar 12, 2020

Choose a reason for hiding this comment

siju-samuel Mar 12, 2020

Choose a reason for hiding this comment

zhiics left a comment

Choose a reason for hiding this comment

siju-samuel left a comment

Choose a reason for hiding this comment

zhiics commented Mar 12, 2020

tqchen commented Mar 11, 2020 •

edited

Loading