-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
adding Relu in raw_ops tensorflow frontend #4780
Conversation
close #4779 |
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.
Small changes needed, and there are some lint (formatting) errors. If you could fix these up and provided all relevant tests are passing this will be good to merge :)
@@ -142,3 +142,6 @@ def Transpose(*, x, perm, name="Transpose"): | |||
|
|||
def ZerosLike(*, x, name="ZerosLike"): | |||
return ivy.zeros_like(x) | |||
|
|||
def Relu(features, name=None): |
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.
name should be "Relu" just to match the rest of the functions in this module
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.
Hello, @jkeane508 , thank you very much for the review. I've replaced the name with "Relu". Although "lint / check-formatting (pull_request)" checks were successful, I am not sure exactly why "test-frontend-tensorflow / run-nightly-tests (pull_request)" checks were failing.
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.
Yea I've looked at the test errors, seems to be an issue with the num_positional_args in the test. I would suggest hardcoding the number of positional args rather than using the helper and see if this helps. I'm aware that currently there is an issue with this
ivy_tests/test_ivy/test_frontends/test_tensorflow/test_raw_ops.py
Outdated
Show resolved
Hide resolved
ivy_tests/test_ivy/test_frontends/test_tensorflow/test_raw_ops.py
Outdated
Show resolved
Hide resolved
ef9f6c9
to
b36eb0c
Compare
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, provided tests are passing will merge once you resolve conflicts :)
b36eb0c
to
82124aa
Compare
No description provided.