Skip to content
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

Closed
wants to merge 0 commits into from

Conversation

kcsayem
Copy link
Contributor

@kcsayem kcsayem commented Sep 22, 2022

No description provided.

@kcsayem
Copy link
Contributor Author

kcsayem commented Sep 22, 2022

close #4779

@ivy-leaves ivy-leaves added the TensorFlow Frontend Developing the TensorFlow Frontend, checklist triggered by commenting add_frontend_checklist label Sep 22, 2022
Copy link
Contributor

@iamjameskeane iamjameskeane left a 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):
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

@iamjameskeane iamjameskeane Sep 23, 2022

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

@kcsayem kcsayem force-pushed the tensor_instance_reshape branch from ef9f6c9 to b36eb0c Compare September 23, 2022 18:12
Copy link
Contributor

@iamjameskeane iamjameskeane left a 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 :)

@kcsayem kcsayem closed this Sep 24, 2022
@kcsayem kcsayem force-pushed the tensor_instance_reshape branch from b36eb0c to 82124aa Compare September 24, 2022 11:30
@iamjameskeane iamjameskeane linked an issue Sep 26, 2022 that may be closed by this pull request
@iamjameskeane iamjameskeane removed a link to an issue Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TensorFlow Frontend Developing the TensorFlow Frontend, checklist triggered by commenting add_frontend_checklist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants