-
Notifications
You must be signed in to change notification settings - Fork 55
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
add inplace pointwise operators #224
base: master
Are you sure you want to change the base?
Conversation
59f04ce
to
8795e7a
Compare
…ark_dirty); add tests for inplace unary pointwise operations
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.
LGTM
I have to solve the problem at when the output has in-appropriate dtype or shape it does not raise error. |
@@ -37,4 +37,16 @@ def add(A, B, *, alpha=1): | |||
elif isinstance(B, torch.Tensor): | |||
return add_func_scalar_tensor(A, B, alpha) | |||
else: | |||
return A + B * alpha | |||
return torch.tensor(A + B * alpha) |
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.
why we need this?
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.
torch returns a tensor for that case.
@@ -32,6 +32,21 @@ def test_accuracy_abs(shape, dtype): | |||
gems_assert_equal(res_out, ref_out) | |||
|
|||
|
|||
@pytest.mark.inplace |
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.
what is inplace
mark used for? can we merge abs_
test into abs
test?
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.
LGTM, is it necessary to add benchmark for those inplace ops?
It'll add a lot of redundancy if we include all the variants. Shall we do that later? |
It's probably a good time to update this PR? |
PR Category
Operator
Type of Change
New Feature
Description
Add inplace pointwise operators.
Issue
Progress
Performance