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: Added hfftn #23540

Closed
wants to merge 3 commits into from
Closed

feat: Added hfftn #23540

wants to merge 3 commits into from

Conversation

ptdatta
Copy link

@ptdatta ptdatta commented Sep 13, 2023

PR Description

Added hfftn to paddle frontend

Related Issue

Close #23457

Checklist

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

Socials:

https://twitter.com/parthibdatta123

@ivy-leaves ivy-leaves added PaddlePaddle Backend Developing the Paddle Paddle Backend. Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist Ivy Functional API Ivy API Experimental Run CI for testing API experimental/New feature or request labels Sep 13, 2023
@ivy-leaves
Copy link

If you are working on an open task, please edit the PR description to link to the issue you've created.

For more information, please check ToDo List Issues Guide.

Thank you 🤗

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!

@ptdatta ptdatta changed the title Added #23457 Added hfftn Sep 13, 2023
@ptdatta ptdatta changed the title Added hfftn feat: Added hfftn Sep 14, 2023
@ptdatta
Copy link
Author

ptdatta commented Sep 14, 2023

@djl11 @vedpatwardhan @Ishticode kindly review this PR. Thank you.

@vedpatwardhan
Copy link
Contributor

vedpatwardhan commented Oct 2, 2023

Given that @Ishticode is assigned to the PR, he'll take a look soon, sorry for the delay @ptdatta 😄

Copy link
Contributor

@Ishticode Ishticode left a comment

Choose a reason for hiding this comment

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

Hi @ptdatta
Thanks for the PR. Currently the PR calls upon ivy.hfftn where ivy does not yet implement hfftn for the backends. This is why test_paddle_hfftn seems to fail as a new failure as seen in the logs. Please either compose the function using existing ivy functions or make a proposal for hfftn to be added as a function in all backends as a ToDo as seen here #3856.

Let me know if unsure. Thanks 🙂️

@ptdatta
Copy link
Author

ptdatta commented Oct 5, 2023

@Ishticode But I have added the backend for this function at ivy/functional/backends/paddle/experimental/layers.py

@with_supported_dtypes(
    {
        "2.5.1 and below": (
            "complex64",
            "complex128",
        )
    },
    backend_version,
)
def hfftn(
    x: paddle.Tensor,
    s: Optional[Union[int, Tuple[int]]] = None,
    axes: Optional[Union[int, Tuple[int]]] = None,
    *,
    norm: Optional[str] = "backward",
    out: Optional[paddle.Tensor] = None,
) -> paddle.Tensor:
    result = paddle.fft.hfftn(x, s, axes, norm)
    return result

Am I missing something?

@ptdatta
Copy link
Author

ptdatta commented Oct 12, 2023

@Ishticode But I have added the backend for this function at ivy/functional/backends/paddle/experimental/layers.py

@with_supported_dtypes(
    {
        "2.5.1 and below": (
            "complex64",
            "complex128",
        )
    },
    backend_version,
)
def hfftn(
    x: paddle.Tensor,
    s: Optional[Union[int, Tuple[int]]] = None,
    axes: Optional[Union[int, Tuple[int]]] = None,
    *,
    norm: Optional[str] = "backward",
    out: Optional[paddle.Tensor] = None,
) -> paddle.Tensor:
    result = paddle.fft.hfftn(x, s, axes, norm)
    return result

Am I missing something?

@vedpatwardhan @Ishticode please reply??

Copy link
Contributor

@Ishticode Ishticode left a comment

Choose a reason for hiding this comment

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

Hi @ptdatta
As already pointed out the majority of backends are missing the implementation. You do have implementation for paddle backend which is good but other backends don't have it. This leads to recursion error where you call current_backend.hfftn it keeps going back onto itself.

When a frontend function is implemented, we expect the ivy... calls to be able to run with any of our supported backends including tf, torch jax and paddle.

Please check our contributing guidelines if these things don't make sense. Thank you very much.

@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.

@ivy-seed ivy-seed added the Stale label Mar 11, 2024
@ivy-seed ivy-seed closed this Mar 18, 2024
@ivy-seed
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ivy API Experimental Run CI for testing API experimental/New feature or request Ivy Functional API Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist PaddlePaddle Backend Developing the Paddle Paddle Backend. Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hfftn
7 participants