[DOC] Add doc for Relay op strategy #5078
Conversation
There was a problem hiding this comment.
Thanks for the document. I feel it might be vague for new users who attempt to add operators to Relay after reading the code example (e.g., "my_new_op"). Maybe it's better to use a real op that has been already well-defined in Relay, so that it's more like you are explaining how one of the well-implemented ops was done. In this case, you just need to copy paste the code snippet from Relay op strategy and refer readers to the real code. Users can also copy paste the required functions and modify them for their new ops.
docs/dev/relay_op_strategy.rst
Outdated
|
|
||
| .. code:: python | ||
|
|
||
| def add_implementation(self, compute, schedule, name="default", plevel=10) |
There was a problem hiding this comment.
The API here seems a bit confuse. Specifically, it doesn't use any keywords mentioned in the above paragraph. My feeling is this section focuses on components on C++ backend, so would that be better to move this API intro to the next section along with other Python interfaces?
There was a problem hiding this comment.
I agree with sticking to the keywords here
There was a problem hiding this comment.
Moved to the later part.
There was a problem hiding this comment.
@icemelon9 awesome doc overall! I left some comments; and agree with @comaniac about trying to make the paragraphs and API use consistent terminology overall. I think that the fact that this also applies to non-symbolic input shapes should be stated upfront rather than in the last paragraph.
docs/dev/relay_op_strategy.rst
Outdated
|
|
||
| .. code:: python | ||
|
|
||
| def add_implementation(self, compute, schedule, name="default", plevel=10) |
There was a problem hiding this comment.
I agree with sticking to the keywords here
|
@comaniac I replaced "my_new_op" with some real ops now. Could you take a look again? |
docs/dev/relay_op_strategy.rst
Outdated
| Register Strategy for An Operator | ||
| --------------------------------- | ||
|
|
||
| There are three methods to register a strategy function for an operator, |
There was a problem hiding this comment.
Some thoughts to this section:
-
It's better to define "strategy function" first. Something like "a strategy function is used to determine which pair of compute and schedule function should be used by given a workload".
-
The third method described at the end of this section (
add_implement) is not a method of registering a strategy function to me. Instead, it is more like how to implement a strategy function. It might be better to have a separate section called "Implement a Strategy for An Operator", or just change this section title to "Implement and Register a Strategy for An Operator". -
My understanding is that the first approach is a simple one because we can reuse the compute function for those patterns. In this case, it might be better to switch the order of first and second methods. That is, we first introduce a method of implementing and registering a strategy function for dense. After that, we mention if your operator fits certain patterns you can simplify your work.
-
In the description of strategy function implementation, I think it might be better to mention
topkonly. As a result, in the beginning of the next section "Advanced Strategy Function", you can say some operators like dense require a more complicate strategy so it cannot be implemented liketopk.
|
I have thought about @comaniac comments and they make a lot of sense. I plan to separate the "Register Strategy for An Operator" into two parts, first describe how to define a strategy function, and second how to do the registration. I'll work on the update today. So let's keep the PR open for now. |
|
made an update. @comaniac @tmoreau89 could you take a look again? |
|
Nice this tutorial has better clarity now. |
|
Thanks @icemelon9 @comaniac the PR has been merged. |
* [DOC] Add doc for Relay op strategy * update * address more comments * update * update
* [DOC] Add doc for Relay op strategy * update * address more comments * update * update
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 by @ them in the pull request thread.
cc @tqchen @kevinthesun @comaniac @tmoreau89 @yzhliu