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

feat: implement rfftn to Paddle Frontend (#23484) #23537

Closed
wants to merge 7 commits into from

Conversation

Kilvny
Copy link

@Kilvny Kilvny commented Sep 13, 2023

PR Description

added rfftn to Paddle Frontend and added tests related to it

Related Issue

(#23484)

Close #23484

Checklist

  • Did you add a function?
  • Did you add the tests?
  • Did you follow the steps we provided?

Socials:

@Kilvny Kilvny requested a review from KareemMAX as a code owner September 13, 2023 15:09
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 Passed!

@ivy-leaves ivy-leaves added the Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist label Sep 13, 2023
@Kilvny Kilvny changed the title added rrftn to Paddle Frontend (#23484) feat: implement rrftn to Paddle Frontend (#23484) Sep 13, 2023
@Kilvny
Copy link
Author

Kilvny commented Sep 13, 2023

Why these 2 check are failing, can you help me with that?

@Kilvny Kilvny changed the title feat: implement rrftn to Paddle Frontend (#23484) feat: implement rfftn to Paddle Frontend (#23484) Sep 13, 2023
Copy link
Contributor

@KareemMAX KareemMAX left a comment

Choose a reason for hiding this comment

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

Hey @Kilvny, seems like you have edited the demos submodule by mistake. Can you please revert that?

@Kilvny
Copy link
Author

Kilvny commented Sep 14, 2023

Hey @Kilvny, seems like you have edited the demos submodule by mistake. Can you please revert that?

Done @KareemMAX

@Kilvny
Copy link
Author

Kilvny commented Sep 15, 2023

@Mr-Niraj-Kulkarni

s = x.shape

# Apply rfft along the last axis
rfft_result = ivy.rfftn(x, s=s, axes=axes, norm=norm, name=name)
Copy link
Contributor

@Mr-Niraj-Kulkarni Mr-Niraj-Kulkarni Sep 18, 2023

Choose a reason for hiding this comment

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

TypeError: rfftn() got an unexpected keyword argument 'name'. This line is throwing an error. there is no name
parameter in rfftn please remove it and add out=name

Copy link
Author

Choose a reason for hiding this comment

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

Thank you, the change is done,
FYA

Copy link
Contributor

@YushaArif99 YushaArif99 left a comment

Choose a reason for hiding this comment

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

Hi @Kilvny

Apologies for the delay in review. Have been a little swamped with work recently and so havent had the time to review any PRs 😅

With regards to the PR itself, the implementation does look good overall. However, I am noticing that the CI tests are failing here with the following error:

E   ivy.utils.exceptions.IvyBackendException: tensorflow: rfftn: Invalid FFT input: `x` must be of a real dtype. Received: <dtype: 'complex128'>

Could you kindly fix the merge conflicts and look into fixing the above issue? Fee free to re-request a review once that's done? Thanks!😄

@Kilvny
Copy link
Author

Kilvny commented Nov 26, 2023

YushaArif99

Hi @YushaArif99 Thank you so much for the reply,
Apologies for taking so long to work on this changes,
I was pretty busy and I got a new job recently

anyways, I resolved the conflicts, but regarding the checks that fails, I need some help and guidance I don't know exactly what causes the tests to fail, your help is highly appreciated

@KareemMAX

@YushaArif99 YushaArif99 requested a review from NripeshN December 4, 2023 05:55
@ivy-seed
Copy link

ivy-seed commented Jan 1, 2024

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.

@KareemMAX KareemMAX removed their request for review January 3, 2024 10:57
@ivy-seed
Copy link

This PR has been closed because it has been marked as stale for more than 7 days with no activity.

@ivy-seed ivy-seed closed this Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rfftn
9 participants