Relaxing type requirements for broadcast_like #17977
Conversation
|
Hey @tobecontinued , Thanks for submitting the PR
CI supported jobs: [centos-gpu, unix-gpu, miscellaneous, website, edge, unix-cpu, windows-gpu, clang, centos-cpu, windows-cpu, sanity] Note: |
|
@mxnet-label-bot add [Operator, pr-awaiting-review] |
| std::vector<int> *out_attrs) { | ||
| CHECK_EQ(in_attrs->size(), 2) << " in operator " << attrs.name; | ||
| std::vector<int> checked_in_attrs = { (*in_attrs)[0] }; | ||
| bool ret = !type_is_none((*in_attrs)[1]) && |
There was a problem hiding this comment.
Thanks for your contribution!
Is it necessary for the condition !type_is_none((*in_attrs)[1]) ?
There was a problem hiding this comment.
Well, I actully just found the issue, the code is copied from merged PR #14097, which had a similar issue. I think it is better to keep the same assumpation for all *_like op need to relax type requirment.
There was a problem hiding this comment.
Yes, it is necessary - what FInferType returns is whether it succeeded in inferring all the types (so that if all operators return true we know that all types are inferred). That is why it is important to not lie and return true only if all types are really inferred (even if we do not actually do anything with the other type).
|
@mxnet-bot run ci [unix-gpu] |
|
Jenkins CI successfully triggered : [unix-gpu] |
|
@mxnet-bot run ci [unix-gpu] |
|
Jenkins CI successfully triggered : [unix-gpu] |
…to broadcast_like_type
|
@mxnet-bot run ci [unix-gpu] |
|
Jenkins CI successfully triggered : [unix-gpu] |
|
@mxnet-bot run ci [all] |
|
Jenkins CI successfully triggered : [unix-gpu, sanity, centos-cpu, unix-cpu, windows-gpu, windows-cpu, clang, website, edge, centos-gpu, miscellaneous] |
|
@mxnet-bot run ci [all] |
|
Jenkins CI successfully triggered : [unix-gpu, sanity, unix-cpu, centos-cpu, centos-gpu, miscellaneous, windows-gpu, edge, clang, windows-cpu, website] |
|
@mxnet-bot run ci [windows-gpu] |
|
Jenkins CI successfully triggered : [windows-gpu] |
|
The PR has been merged. Thanks for your contribution! : ) |
* Relaxing type requirements for broadcast_like * enhance unit test
* Relaxing type requirements for broadcast_like * enhance unit test
* Relaxing type requirements for broadcast_like * enhance unit test
…19447) * Relaxing type requirements for broadcast_like * enhance unit test Co-authored-by: wicky <tbc.dengwenqi@gmail.com>
Description
Similarly to #14097, broadcast_like op should also not dictate the type of its second argument (since only shape metadata is used there).
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.