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(frontends): added the std method to paddle frontend #22152

Merged
merged 17 commits into from
Sep 24, 2023

Conversation

samthakur587
Copy link
Contributor

close #22151

@github-actions
Copy link
Contributor

Thanks for contributing to Ivy! 😊👏
Here are some of the important points from our Contributing Guidelines 📝:
1. Feel free to ignore the run_tests (1), run_tests (2), … jobs, and only look at the display_test_results job. 👀 It contains the following two sections:
- Combined Test Results: This shows the results of all the ivy tests that ran on the PR. ✔️
- New Failures Introduced: This lists the tests that are passing on main, but fail on the PR Fork. Please try to make sure that there are no such tests. 💪
2. The lint / Check formatting / check-formatting tests check for the formatting of your code. 📜 If it fails, please check the exact error message in the logs and fix the same. ⚠️🔧
3. Finally, the test-docstrings / run-docstring-tests check for the changes made in docstrings of the functions. This may be skipped, as well. 📚
Happy coding! 🎉👨‍💻

@ivy-leaves ivy-leaves added the Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist label Aug 18, 2023
@samthakur587
Copy link
Contributor Author

hey @mohame54 can you please check this PR.I am not getting any error in local by testing it.

@ivy-seed ivy-seed assigned hmahmood24 and unassigned mohame54 Aug 28, 2023
@samthakur587
Copy link
Contributor Author

hey @hmahmood24 can you please review the PR. and let me know whether any changes required or not. So i can move to the next task. 🚀

Copy link
Contributor

@hmahmood24 hmahmood24 left a comment

Choose a reason for hiding this comment

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

Hey @samthakur587, thanks a lot for the spot and this PR! I have suggested some minor changes. Let me know once you're done with them. Thanks! :)

Comment on lines 649 to 651
ivy.std(
self._ivy_array, axis=axis, correction=int(unbiased), keepdims=keepdim
)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a std function in paddle_frontend now. We should use that here instead of ivy.std. The resulting code would look something like:

def std(self, axis=None, unbiased=True, keepdim=False, name=None):
        return paddle_frontend.std(self, axis=axis, unbiased=unbiased, keepdim=keepdim)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done , made the changes required i think now its ready to merge. 🚀

thank you

Copy link
Contributor

@hmahmood24 hmahmood24 left a comment

Choose a reason for hiding this comment

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

@samthakur587 The tests are failing. I think because you're using a strategy from some other module to generate the inputs for your function which doesn't allow you to generate the valid dtypes that your function is valid for.

@samthakur587
Copy link
Contributor Author

@samthakur587 The tests are failing. I think because you're using a strategy from some other module to generate the inputs for your function which doesn't allow you to generate the valid dtypes that your function is valid for.

hey @hmahmood24 i have tried to debug the test function and changed my composite _statistical_dtype_values the input dtype("valid") but still the numpy is taking the float16 dtype . at my last commit i checked that all test are passing but now i think some changed happend in numpy testing. thats why numpy not passing but others backend are passing.

same thing is happening in the stat.py std function that i am calling here as paddle frontend also in Var method and etc.

image

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!

@samthakur587
Copy link
Contributor Author

hey @hmahmood24 i have tried to debug the ivy.std and i changed the composite function __statistical_dtype_values the helper dtype to ('valid') then all the backend are not passing bcz i have seen that the std at the paddle stat fronted include the unit8 and same as tensorflow and numpy so they returned the error as arrestion with dtype .

i am not found the actual bug that why it taking all float16 even i taken the valid dtype. can you please check.

image

Copy link
Contributor

@hmahmood24 hmahmood24 left a comment

Choose a reason for hiding this comment

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

Hey @samthakur587, thanks for the update. Can you try removing the with_supported_dtypes decorator altogether and see if that works for you? Since this std method calls the paddle_frontend.std function which already has the with_supported_dtypes decorator applied to it here. Also, no need to change the _statistical_dtype_values since that is being used in numerous places so we shouldn't really be modifying it

@samthakur587
Copy link
Contributor Author

Hi! as you said i have tried with removing the decorator @with_unsupported_dtypes({"2.5.1 and below": ("float16", "bfloat16")}, "paddle"). but its not worked. at the paddle stat.py the std function that is implemented is supporting the ("float64","float32","uint16") and uint16 not supported as mention in the paddle doc. and falling at all the backend with the dtype "bfloat16".

image

So i think we should keep the decorator on the tensor.std method.

@samthakur587
Copy link
Contributor Author

I tried to check the ivy.std function. by running the tests at pytest -v ivy_tests/test_ivy/test_functional/test_core/test_statistical.py::test_std. it also falling in all the backend so i added the unsupported_dtype("bfloat","float16") after that all the backend passed but the numpy is not passing even i have also applied the same decortor to numpy backend.

@_scalar_output_to_0d_array
@with_unsupported_dtypes({"1.26.0 and below": ("bfloat16","float16")}, backend_version)
def std(
    x: np.ndarray,
    /,
    *,
    axis: Optional[Union[int, Sequence[int]]] = None,
    correction: Union[int, float] = 0.0,
    keepdims: bool = False,
    out: Optional[np.ndarray] = None,
) -> np.ndarray:
    axis = tuple(axis) if isinstance(axis, list) else axis
    return ivy.astype(np.std(x, axis=axis, ddof=correction, keepdims=keepdims, out=out),x.dtype,copy=False)


std.support_native_out = True

I don't konw but numpy still supporting the bfloat16 dtype. in the ivy.std also the numpy is not passing and returning the same error bfloat16 not supporting. here i have atteched the result.

image

image

@samthakur587
Copy link
Contributor Author

Hi! @hmahmood24

Now its passing all the test with all the backend. and ready to merge 🚀

image

@hmahmood24 hmahmood24 changed the title added the std method to paddle fronted feat(frontends): added the std method to paddle frontend Sep 23, 2023
Copy link
Contributor

@hmahmood24 hmahmood24 left a comment

Choose a reason for hiding this comment

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

Hey @samthakur587, looks good now. Can you just fix the lint issues with the PR so we can go ahead and merge it? Thanks!

Copy link
Contributor

@hmahmood24 hmahmood24 left a comment

Choose a reason for hiding this comment

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

@samthakur587 Everything looks good to merge. Thanks! 🚀

@hmahmood24 hmahmood24 merged commit cf56635 into ivy-llc:main Sep 24, 2023
101 of 135 checks passed
iababio pushed a commit to iababio/ivy that referenced this pull request Sep 27, 2023
@samthakur587 samthakur587 deleted the std branch October 1, 2023 21:11
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.

ivy.functional.frontends.paddle.tensor.tensor in std method
4 participants