Add view weight norm where zeropad2d global test #7886
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…add_view_weight_norm_where_zeropad2d_global_test
wyg1997
reviewed
Mar 25, 2022
| @@ -86,7 +86,10 @@ Maybe<void> InferWhereXYScalarTensorDesc(user_op::InferContext* ctx) { | |||
| Maybe<void> GetWhereSbpSignatures(user_op::SbpContext* ctx) { | |||
There was a problem hiding this comment.
这个 op 因为 condition、x、y 都是 broadcastable 的，这里的 sbp 推导可能还不完全正确？
There was a problem hiding this comment.
这个 op 因为 condition、x、y 都是 broadcastable 的，这里的 sbp 推导可能还不完全正确？
确实不完善，我在refine一下
There was a problem hiding this comment.
是的，这里的推导逻辑会比较复杂，看是不是可以简化一下。比如只有condition、x、y shape相同时才支持所有输入的split，如果有两个相同，另一个不同，那就参考broadcast_add那里的逻辑，如果三个都不同，那就只支持B
…add_view_weight_norm_where_zeropad2d_global_test
…tps://github.com/Oneflow-Inc/oneflow into add_view_weight_norm_where_zeropad2d_global_test
clackhan
commented
Mar 28, 2022
oneflow/user/ops/where_op.cpp
Outdated
Comment on lines
28
to
32
| CHECK_OR_RETURN(a_extend_shape.At(i) == 1 || b_extend_shape.At(i) == 1 | ||
| || a_extend_shape.At(i) == b_extend_shape.At(i)) | ||
| << Error::RuntimeError() << "The size of tensor a (" << a_extend_shape.At(i) | ||
| << ") must match the size of tensor b (" << b_extend_shape.At(i) | ||
| << ") at non-singleton dimension " << i; |
hjchen2
reviewed
Mar 28, 2022
oneflow/user/ops/where_op.cpp
Outdated
Comment on lines
54
to
56
| JUST(CheckBroadcastable(a_shape, b_shape)); | ||
| JUST(CheckBroadcastable(a_shape, c_shape)); | ||
| JUST(CheckBroadcastable(b_shape, c_shape)); |
There was a problem hiding this comment.
这里的check没太大必要，因为在InferWhereTensorDesc 里实际上已经check过一遍了
There was a problem hiding this comment.
这里的check没太大必要，因为在
InferWhereTensorDesc里实际上已经check过一遍了
确实没有必要，已删除CheckBroadcastable相关逻辑
hjchen2
approved these changes
Mar 28, 2022
wyg1997
reviewed
Mar 28, 2022
wyg1997
approved these changes
Mar 28, 2022
Speed stats: |
|
View latest API docs preview at: https://staging.oneflow.info/docs/Oneflow-Inc/oneflow/pr/7886/ |
|
CI failed when running job: cpu-misc. PR label automerge has been removed |
clackhan
requested review from
oneflow-ci-bot
and removed request for
oneflow-ci-bot
|
View latest API docs preview at: https://staging.oneflow.info/docs/Oneflow-Inc/oneflow/pr/7886/ |
Speed stats: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.