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 inverse fast fourier transform (irfftn) function for paddle frontend #23526

Conversation

xingshuodong
Copy link
Contributor

@xingshuodong xingshuodong commented Sep 13, 2023

PR Description

feat: implement inverse fast Fourier transform (irfftn) function for paddle frontend

Add paddle.fft Fast Fourier Transform Functions to Paddle Frontend #15047
This pull request is related to resolves [irfftn #22913].

Checklist

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

Socials:

[email protected]
ins: xingshuodong

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!

@xingshuodong
Copy link
Contributor Author

@tomatillos please reply me at this PR, i think the old PR is going to be closed.
As far as the irfftn is working for n-dim case, please let me know if I need to do some improvement.

@ivy-leaves ivy-leaves added the Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist label Sep 13, 2023
…transform-(irfftn)-function-for-paddle-frontend
@tomatillos
Copy link
Contributor

Also fix the lint and PR compliance checks please.

@mobley-trent
Copy link
Contributor

@xingshuodong this is a great PR ⚡
Just resolve the minor issues and this should be good to merge.
Is there a valid reason why you've set s to None in the tests ? There might be a bug in your code you aren't aware about

@mobley-trent
Copy link
Contributor

As well as axes

change the test case to axis, and the norm to backward or forward or...
@xingshuodong
Copy link
Contributor Author

xingshuodong commented Sep 14, 2023

Also fix the lint and PR compliance checks please.

@tomatillos Hi Tom, i cant pass the lint check becasue the file itself can't pass the lint check specific to rfftfreq,
should i modify it?

@xingshuodong xingshuodong changed the title Feat implement inverse fast fourier transform (irfftn) function for paddle frontend feat: implement inverse fast fourier transform (irfftn) function for paddle frontend Sep 14, 2023
@xingshuodong
Copy link
Contributor Author

@xingshuodong this is a great PR ⚡ Just resolve the minor issues and this should be good to merge. Is there a valid reason why you've set s to None in the tests ? There might be a bug in your code you aren't aware about

@mobley-trent Hi Eddy, during learning of development, as there is a general rule for manuplate the shape of x tensor, none is a better choice

The valid reason that here should set to be none, is the setup method for frontend test is only supporting the axis but not axes, the frontend is expecting the single axis as test input rather than the axes, this is out scope with the specific use of s.
so the functionality of irfftn will support range of axes as input (it works in loop), it will always work for s=None, the extra step can be add the 0 pad as following describtion

Take example of input a tensor x which is 3 dim with 2 dim size, (2, 2, 2)
Same as Input: If you want the output to be of the same shape as the input, you could specify s=(2, 2, 2).

irfftn(my_tensor, s=(2, 2, 2), axes=(0, 1, 2))
Different Size: If you want to zero-pad or truncate along specific dimensions, you can specify different sizes.

irfftn(my_tensor, s=(3, 3, 3), axes=(0, 1, 2))
This will zero-pad the frequency-domain data before transforming, giving you a (3, 3, 3) output tensor.

Specific Axes: If you want to transform along specific axes but not others, specify the axes and shape accordingly.

irfftn(my_tensor, s=(3, 3), axes=(0, 1))
This will perform the inverse FFT along the first two axes and zero-pad to (3, 3) along those axes, leaving the third axis unchanged.

Fix the irfftn with mutiple dims case, specific the axe is none, apply all axes
test case for apply all axes to irfftn
@xingshuodong
Copy link
Contributor Author

xingshuodong commented Sep 15, 2023

@mobley-trent Hi Eddy I have carefully review the irfftn explnation, and rewrite the function.

  1. for nomral axes, irfftn use ifftn
  2. for last axis, irfftn use the developed calculation represents irfft

Now this is a true irfftn function for any dims any size, it enable:

  1. input axes like [0, 2, 3, 4] or [int] or None
  2. input shape as None, or a customerized shape, the ifftn function used in irfftn is supporting this function

@xingshuodong
Copy link
Contributor Author

xingshuodong commented Sep 15, 2023

image
In Tensorflow, in some cases the assert_all_close function is resulting issue of result doesn't match, this is an backend issue, but may occur during irfftn. ifftn in jax only support 3 D


image

…transform-(irfftn)-function-for-paddle-frontend
@mobley-trent
Copy link
Contributor

Hey @xingshuodong your function should at least pass for the minimal example I provided earlier from the paddle docs

@xingshuodong
Copy link
Contributor Author

xingshuodong commented Sep 19, 2023

Hey @xingshuodong your function should at least pass for the minimal example I provided earlier from the paddle docs

it does pass, you can check the picture, what i mean is the test method fixed doesn't mean it works in limited case, it depends on how the backend developed in other backend, just like in tensorflow there is no nature irfftn function, so it is better to have irfftn special for tensorflow itself, or just rasie the error.

Please check this picture, it works for most cases, not only limit one. @mobley-trent , this function is work well for numpy and paddle, for those who dont have ifftn support N-D, it will only work for 3-D.
image

@xingshuodong
Copy link
Contributor Author

xingshuodong commented Sep 19, 2023

Please pay attention to check, it is tested. @tomatillos, it should meet the irfftn performance for paddle, becasue it can work in paddle.

@mobley-trent
Copy link
Contributor

Only the paddle backend seems to be running in this case. We can just merge for now
Thanks again for your PR @xingshuodong 🚀

@mobley-trent mobley-trent merged commit 1bd6928 into ivy-llc:main Sep 19, 2023
125 of 135 checks passed
@xingshuodong
Copy link
Contributor Author

The case here is this irfftn is working correctly in numpy backend and paddle backend.
Jax and tensorflow backend with over 3 - D dims, need some specific consideration.

Dhruv-Varshney-developer added a commit to Dhruv-Varshney-developer/ivy that referenced this pull request Sep 20, 2023
iababio pushed a commit to iababio/ivy that referenced this pull request Sep 27, 2023
druvdub pushed a commit to druvdub/ivy that referenced this pull request Oct 14, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

irfftn
4 participants