Enhancements for custom subgraph op#17194
Conversation
|
@mxnet-label-bot add [pr-awaiting-review] |
|
@TaoLv @ZhennanQin would you please review since you're familiar with the subgraph API? |
| create_opstate = func; | ||
| return *this; | ||
| } | ||
| CustomOp& isSubgraphOp(bool state) { |
There was a problem hiding this comment.
Sounds this function should return true or false. How about using setXXX?
There was a problem hiding this comment.
done! changed it to setIsSubgraphOp() and removed argument
| DefaultSubgraphOpStorageType, plevel); | ||
| regOp.set_attr<FResourceRequest>("FResourceRequest", | ||
| DefaultSubgraphOpResourceRequest, plevel); | ||
| regOp.set_attr<nnvm::FMutateInputs>("FMutateInputs", |
There was a problem hiding this comment.
should we check if (mutate_fp != nullptr) here?
There was a problem hiding this comment.
as discussed offline, we'll only use default functions when the user sets the setIsSubgraphOp flag. Later we can revisit if the user wants to use custom functions mixed with default functions
|
Hey @wkcn this PR is finished and ready for review, would you please take a look into this enhancement for custom subgraph ops? Happy New Year! |
|
@samskalicky Happy New Year! This PR looks good to me : ) |
The current subgraph op has an example tests here, but is SUPER hacky: It will be officially tested in test_extensions.py in the other PR that adds support for custom subgraph properties and actually uses custom subgraph ops in #17034 here are the code changes: This PR (#17194) modifies the custom op that is used in that test with the changes in this PR: Would be great to get your feedback on #17034 too. If this is acceptable would you please approve/merge this PR? |
Description
Enhancements for custom subgraph operators in external libraries.
Design
Its difficult to implement the full shape/type propagation in an external library for a whole subgraph. Rather that re-implement these complex pieces, we'll use the default functions in the subgraph API.
Starting from the user in the custom library, the registration is now only:
Here, setting
isSubgraphOpmeans we dont have to register the infer shape/type functions. When re-registering the operator in MXNet, ifisSubgraphOpis true, then we re-use the functions from the subgraph API:Checklist
Essentials
Please feel free to remove inapplicable items for your PR.