-
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
feat(frontends): Implement rfft in PaddlePaddle frontend and fix fft for Tensorflow backend #19454
Conversation
Still working on fixing the failing tests for tensorflow and torch. |
@hmahmood24 Can you please have a look at the implementation here. I can't get the tensorflow and torch tests to pass. |
The torch tests seem to be passing now but still having issues with tensorflow. The main error is that the elements list is empty so it throws the error cannot draw from an empty list. At first I thought it was because of the norm list that I am generating but it doesn't seem like its related to it. |
dtype_input_axis=helpers.dtype_values_axis( | ||
available_dtypes=helpers.get_dtypes("float"), | ||
shape=(2,), | ||
min_axis=-1, | ||
force_int_axis=True, | ||
), |
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 can't we use something like this?
dtype_input_axis=helpers.dtype_values_axis(
available_dtypes=helpers.get_dtypes("valid"),
min_num_dims=1,
min_dim_size=2,
shape=helpers.get_shape(...),
force_int_axis=True,
valid_axis=True,
allow_neg_axis=True,
)
force_int_axis=True, | ||
), | ||
norm=st.sampled_from(["backward", "ortho", "forward"]), | ||
n=st.integers(min_value=2, max_value=10), |
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.
We also need to test for the default n=None
case
…is setup to accommodate valid data types, adjustable axis
Hi @hmahmood24 sorry for the late update but please take a look. |
max_num_dims=2, | ||
min_dim_size=2, | ||
max_dim_size=4,), |
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.
@asad0991 Perhaps you forgot to install/run pre-commit
to fix your lint/formatting errors?
@hmahmood24 Fixed the lint issues I changed devices forgot to install pre-commit on the new device. |
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.
@AwkNinja I can see a couple of failing tests still associated with this PR
@hmahmood24 The tests for tensorflow gives me keyError: "cup" its like the error you get when the version is wrong in the decorator but I used the correct one. For the paddle error it just started to occur just now. Its basically in ivy.dft function I'll take a look at it. |
Pretty sure you can find a solution to that key error problem with a simple search in discord since that's been brought up numerous times already |
I did read on discord changing version to 2.5.1 and below should have fixed it. I'll have a look again might be another solution someone brought up. |
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.
PR Compliance Checks Passed!
…due to slight value differences, added conversion from `float` to `complex` as `tensorflow` backend for `fft` doesn't support `float`
Hi @hmahmood24, the |
Hey @AwkNinja, thanks a lot for the effort. I'll ask someone from the team to take a look at this issue you brought up with |
This wasn't a |
Hi @hmahmood24 all the test cases are passing now with the fix that @AnnaTz did. Please take a look. |
…for Tensorflow backend (ivy-llc#19454) Co-authored-by: hmahmood24 <[email protected]>
Close #19383