-
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: gradient numpy frontend #23104
Conversation
Thanks for contributing to Ivy! 😊👏 |
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 🤗 |
@handle_frontend_test( | ||
fn_tree="numpy.gradient", | ||
dtype_input_axis=helpers.dtype_values_axis( | ||
available_dtypes=("float32", "float16", "float64"), |
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.
the available dtypes should be a strategy that gets all valid dtypes, you should then set the supported dtypes for the function using one of the decorators specified here https://unify.ai/docs/ivy/overview/deep_dive/data_types.html#supported-and-unsupported-data-types
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.
please add support for edge_order
argument in testing
varargs=helpers.ints( | ||
min_value=-3, | ||
max_value=3, | ||
), |
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.
the varargs
argument can be one of the following as per the function documentation:
- single scalar to specify a sample distance for all dimensions.
- N scalars to specify a constant sample distance for each dimension.
i.e.dx
,dy
,dz
, ... - N arrays to specify the coordinates of the values along each
dimension of F. The length of the array must match the size of
the corresponding dimension - Any combination of N scalars/arrays with the meaning of 2. and 3.
so please extend the testing strategy to accommodate these cases
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.
okay, i'm working on it. thank you
On adding edge_order argument in testing, an exception is raised stating edge_order should not be greater than axis.shape. For example, if axis=0 and edge_order=1 (default value), there is no error produced but in case of axis=0 and edge_order=2, the said exception is raised. For multidimentional arrays the shape of axis will be greater than 1 and in this case edge_order=2 does not produce any error. |
in case of paddle backend, the following error occurs. ivy.utils.exceptions.IvyValueError: paddle: gradient: paddle: prod: (InvalidArgument) prod(): argument 'x' (position 0) must be Tensor, but got Array (at /paddle/paddle/fluid/pybind/eager_utils.cc:1005) I have tried adding function decorators which convert numpy array to tensor, also tried importing ivy.paddle/ivy.torch in test file and calling the funtion paddle.tensor/torch.to_tensor inside an if block when backend=='paddle' and still could not resolve this error. |
to fix this, you might need to write a more complex testing strategy that samples
can you please share the entire error trace? |
|
The problem is solved now, please sync your branch with main and try again |
Will do, thank you for the update. |
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
Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.
Protected Branch
In order to be considered for merging, the pull request changes must not be implemented on the "main" branch. This is described in our Contributing Guide. We are closing this pull request and we would suggest that you implement your changes as described in our Contributing Guide and open a new pull request.
Thank you for this PR, here is the CI results: Failed tests:This PR introduces the following new failing tests: |
Hi, are you still working on this? |
in my local, all the test cases are passing. I've committed all the changes here. |
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.
@sineha17
Thanks for the PR and following through with the instruction above. I've tested locally and tests are passing. Apologies for the delays in merging on our end. Thank you very much.
added gradient function to numpy frontend
out of 5 test cases, 4 pass successfully. paddle backend function expects a tensor attribute but the numpy gradient function uses an array. i'm unable to convert the array to a tensor element for the paddle test case without affecting the input datatype of the other test cases.
please provide some insight on how to fix this error.
close #23053
Checklist
Socials: