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

fix: failing test for sin #27156

Merged
merged 4 commits into from
Nov 11, 2023
Merged

fix: failing test for sin #27156

merged 4 commits into from
Nov 11, 2023

Conversation

Aaryan562
Copy link
Contributor

@Aaryan562 Aaryan562 commented Oct 28, 2023

failing test for sin (shared priority task) #12656

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR Compliance Checks

Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.

Issue Reference

In order to be considered for merging, the pull request description must refer to a specific issue number. This is described in our contributing guide and our PR template.
This check is looking for a phrase similar to: "Fixes #XYZ" or "Resolves #XYZ" where XYZ is the issue number that this PR is meant to address.

@Aaryan562 Aaryan562 changed the title fix: failing test for sin Fixes: failing test for sin Oct 28, 2023
@Aaryan562 Aaryan562 changed the title Fixes: failing test for sin fix: failing test for sin Oct 28, 2023
@ivy-leaves ivy-leaves added the PaddlePaddle Backend Developing the Paddle Paddle Backend. label Oct 28, 2023
Copy link
Contributor

@hello-fri-end hello-fri-end left a comment

Choose a reason for hiding this comment

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

Hey @Aaryan562, thanks a lot for looking into this failing test. Could you please link this issue in the PR title. Also, I'm not sure if I follow why we shouldn't be testing for gradients in this function, could you please explain why it is necessary to set test_gradients as False in this?

@Aaryan562
Copy link
Contributor Author

Hey @Aaryan562, thanks a lot for looking into this failing test. Could you please link this issue in the PR title. Also, I'm not sure if I follow why we shouldn't be testing for gradients in this function, could you please explain why it is necessary to set test_gradients as False in this?

ivy.utils.exceptions.IvyBackendException: paddle: mean: (NotFound) The kernel with key (CPU, Undefined(AnyLayout), float16) of kernel `mean` is not registered. Selected wrong DataType `float16`. Paddle support following DataTypes: float64, complex128, float32, complex64, bool.
E         [Hint: Expected kernel_iter == iter->second.end() && kernel_key.backend() == Backend::CPU != true, but received kernel_iter == iter->second.end() && kernel_key.backend() == Backend::CPU:1 == true:1.] (at /paddle/paddle/phi/core/kernel_factory.cc:199)

@hello-fri-end Basically when paddle was taking float16 dtype and test_gradients flag was set to True it gave the above error. So in my latest commit I have focused on that but I am not quite sure why is error for this condition. Maybe your inputs could help.

@Aaryan562
Copy link
Contributor Author

@vedpatwardhan Since @hello-fri-end would be inactive this week, could you pls review my PR

Copy link
Contributor

@vedpatwardhan vedpatwardhan left a comment

Choose a reason for hiding this comment

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

lgtm! Feel free to merge, thanks @Aaryan562 😄

@Aaryan562 Aaryan562 merged commit 3678f56 into ivy-llc:main Nov 11, 2023
314 of 417 checks passed
@Aaryan562 Aaryan562 deleted the sin_elementwise branch November 11, 2023 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PaddlePaddle Backend Developing the Paddle Paddle Backend.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants