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

Added tf.diag frontend + test #14057

Merged
merged 7 commits into from
May 17, 2023
Merged

Conversation

mobley-trent
Copy link
Contributor

Closes #13868

@ivy-leaves ivy-leaves added the TensorFlow Frontend Developing the TensorFlow Frontend, checklist triggered by commenting add_frontend_checklist label Apr 12, 2023
@MahmoudAshraf97 MahmoudAshraf97 removed their assignment Apr 19, 2023
@ivy-seed ivy-seed added the Stale label Apr 20, 2023
@ivy-seed
Copy link

This PR has been labelled as stale because it has been inactive for more than 7 days. If you would like to continue working on this PR, then please add another comment or this PR will be closed in 7 days.

@mobley-trent
Copy link
Contributor Author

Hi @zaeemansari70, I am requesting a code review 🙂.
I've implemented the test but I am getting this error:

FAILED ivy_tests/test_ivy/test_frontends/test_tensorflow/test_linalg.py::test_tensorflow_diag[cpu-ivy.functional.backends.numpy-False-False] - AttributeError: 'NoneType' object has no attribute 'backend'
FAILED ivy_tests/test_ivy/test_frontends/test_tensorflow/test_linalg.py::test_tensorflow_diag[cpu-ivy.functional.backends.torch-False-False] - AttributeError: 'NoneType' object has no attribute 'backend'

Does this mean the backends are not yet implemented ?

@fnhirwa fnhirwa removed the Stale label Apr 26, 2023
@mobley-trent
Copy link
Contributor Author

Hey @hirwa-nshuti I implemented the function with the appropriate parameters but I am getting the same error that I got beforehand

@mobley-trent mobley-trent requested a review from fnhirwa May 1, 2023 10:55
@AnnaTz
Copy link
Contributor

AnnaTz commented May 10, 2023

Hi @mobley-trent, I tried running your code locally and I don't get the AttributeError: 'NoneType' object has no attribute 'backend' that you mentioned. Are you getting the same when you try running other tests as well? If so, there is something wrong with your ide setup. Otherwise, I would suggest syncing with the latest master and trying again then.

Copy link
Contributor

@fnhirwa fnhirwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mobley-trent

Good Job so far,
But there is a mismatch between the shapes of returned array and the ground truth one🙂
Would you look into that?

@fnhirwa fnhirwa assigned fnhirwa and unassigned MahmoudAshraf97 May 15, 2023
@mobley-trent
Copy link
Contributor Author

@hirwa-nshuti I actually had a question about the testing. The test case which is failing produces this array:

x = array([[0, 0],[0, 0]], dtype=int64)
k = 0

But when I test this input on the diag function I have implemented and tf.linalg.diag, they both produce the same array with shape (2, 2, 2) yet the error trace for the test failure indicates that my function returns an array of shape (2, 3, 3). How is this so ? I tested the function in a Jupyter notebook to make sure that both the ground truth function and my implementation produce the same outputs.

@mobley-trent mobley-trent requested a review from fnhirwa May 15, 2023 17:23
Copy link
Contributor

@fnhirwa fnhirwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mobley-trent

Now looks great but the tests for paddle backend are failing, fix this then we can merge this PR.

Great job done😊

Copy link
Contributor

@fnhirwa fnhirwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mobley-trent

Thanks for contributing😊
Going to merge

@fnhirwa fnhirwa merged commit 2dab3b0 into ivy-llc:master May 17, 2023
@mobley-trent mobley-trent deleted the ivy-frontends-tf branch May 18, 2023 12:09
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.

diag
6 participants