-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TOPI] Numpy consistency: always broadcast binary op. #1321
Conversation
topi/src/topi.cc
Outdated
@@ -149,10 +148,11 @@ TVM_REGISTER_GLOBAL("topi.negative") | |||
*rv = negative(args[0]); | |||
}); | |||
|
|||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tqchen is there any reason why this is commented out instead of deleted?
verify_broadcast_binary_ele((32,), (64, 32), typ="maximum") | ||
verify_broadcast_binary_ele((1, 2, 2, 1, 32), (64, 32), typ="minimum") | ||
verify_broadcast_binary_ele( | ||
(5, 2, 3), (2, 1), topi.add, np.add) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix indentation
verify_broadcast_binary_ele( | ||
(5, 2, 3), (), topi.subtract, np.subtract) | ||
verify_broadcast_binary_ele( | ||
(5, 2, 3), None, topi.subtract, np.subtract) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good to me. Left minor comments on cosmetics.
bff71d0
to
cd507b1
Compare
@tmoreau89 you might need to update downstream call to broadcast_ops as they are renamed |
List of Operators