-
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): added the std method to paddle frontend #22152
Conversation
Thanks for contributing to Ivy! 😊👏 |
hey @mohame54 can you please check this PR.I am not getting any error in local by testing it. |
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. 🚀 |
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.
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! :)
ivy.std( | ||
self._ivy_array, axis=axis, correction=int(unbiased), keepdims=keepdim | ||
) |
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.
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)
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.
done , made the changes required i think now its ready to merge. 🚀
thank you
updated the std method
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.
@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. |
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!
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. |
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.
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
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". So i think we should keep the decorator on the tensor.std method. |
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. |
Hi! @hmahmood24 Now its passing all the test with all the backend. and ready to merge 🚀 |
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.
Hey @samthakur587, looks good now. Can you just fix the lint issues with the PR so we can go ahead and merge it? Thanks!
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.
@samthakur587 Everything looks good to merge. Thanks! 🚀
Co-authored-by: hmahmood24 <[email protected]>
Co-authored-by: hmahmood24 <[email protected]>
close #22151