Add increment method to Paddle frontend #21369

FelixHuyghe · 2023-08-06T12:36:28Z

github-actions · 2023-08-06T12:36:39Z

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 master, 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! 🎉👨‍💻

hmahmood24

Hey @FelixHuyghe, thanks for the PR! Requesting some changes on the PR. Thank you :)

hmahmood24 · 2023-08-13T09:32:49Z

ivy/functional/frontends/paddle/tensor/math.py

+)
+@to_ivy_arrays_and_back
+def increment(x, value=1.0):
+    if x.size == 1:


x.size returns a list. Maybe you forgot to take its length?

I am a bit confused by this: if I just type in the terminal something like a = ivy.array([3,4,5,6,7] and then do a.size I just get an int 5, which is what I would expect. I want to check if the whole array contains exactly one element.

@FelixHuyghe Why is this if condition needed anyways?

The paddle function accepts only inputs with one element:
paddle.increment(x, value=1.0, name=None) The API is usually used for control flow to increment the data of x by an amount value. Notice that the number of elements in x must be equal to 1.
The problem is that ivy.increment_incplace accepts inputs with any shape, so I thought I check if the size is equal to one and if not I return an exception. However if I try to test this by setting the max_num_size parameter for x to something larger then 1, the tests fail and I get the exception I wrote myself. I was trying to test that when you provide an input array with a size larger than 1 the function indeed raises an exception, just like the original Paddle function. But I don't think I am doing it in the correct way, if you could help me :)
I was also wondering if in this case the frontend function should return the exact same exception as when paddle.increment is called directly. This is why in my previous commit I tried calling paddle.increment directly from the frontend, to get the exact same exception, but I understand this is not the way to go.

The paddle function accepts only inputs with one element: paddle.increment(x, value=1.0, name=None) The API is usually used for control flow to increment the data of x by an amount value. Notice that the number of elements in x must be equal to 1. The problem is that ivy.increment_incplace accepts inputs with any shape, so I thought I check if the size is equal to one and if not I return an exception. However if I try to test this by setting the max_num_size parameter for x to something larger then 1, the tests fail and I get the exception I wrote myself. I was trying to test that when you provide an input array with a size larger than 1 the function indeed raises an exception, just like the original Paddle function. But I don't think I am doing it in the correct way, if you could help me :) I was also wondering if in this case the frontend function should return the exact same exception as when paddle.increment is called directly. This is why in my previous commit I tried calling paddle.increment directly from the frontend, to get the exact same exception, but I understand this is not the way to go.

@FelixHuyghe You don't need to test for values that you already know don't fall into the acceptable range of values for this function. The unit test is required to provide complete coverage of an acceptable range of values and so only values that the original function should run fine on should be generated with the unit tests if that makes sense. :)

hmahmood24 · 2023-08-13T09:34:45Z

ivy_tests/test_ivy/test_frontends/test_paddle/test_tensor/test_math.py

@@ -1864,3 +1864,24 @@ def test_paddle_diff(
        prepend=prepend[0],
        append=append[0],
    )
+
+


You didn't generate any input strategies for the test using the @handle_frontend_test decorator. Kindly add these and run the test using pytest to see if it passes with all backends or not

Thank you, I did not really understand how the testing worked indeed. In my latest commit I have added a @handle_frontend_test but my tests are not working.
For example with input data [0] and value 1, I get the error that my solution and the groundtruth solution not match: [1]!=[2]
After inspecting the testing method, I think that this is caused because the same array is passed to my implementation of the frontend function first and then the ground truth version later. The problem is that a side effect of the padddle.increment value is that the data that you give it is incremented itself. So when the frontend is called with x=[0] and value=1, the correct result [1] is returned, but causes x=[1] which is why when the groundtruth value is calculated afterwards it adds 1 to 1 and that is how the ground truth value ends up being wrong. If I manually change value to 2 for example I get the error [2]!=[4] in the same case (the one produced by the reproduce failure in my code), so it just does the addition twice for the ground truth value. I hope my explanation was understandable, I am a bit lost in how to properly test this method, so hope you can help me!

@FelixHuyghe Try copying the array using something like ivy.copy_array before passing it to ivy.inplace_increment. See if that works for you and let me know :)

Yes that works! I copied the array before passing it to ivy.inplace_increment and now my tests pass (when x is of shape [1] anyways)! However, I think this implementation does not fully reflect the original paddle one, because if you pass an array to the paddle function, a side effect is that array is incremented "in place" and with my frontend implementation when the function returns the provided array is still the same (because of the copy).

Yes that works! I copied the array before passing it to ivy.inplace_increment and now my tests pass (when x is of shape [1] anyways)! However, I think this implementation does not fully reflect the original paddle one, because if you pass an array to the paddle function, a side effect is that array is incremented "in place" and with my frontend implementation when the function returns the provided array is still the same (because of the copy).

I see what you mean. I think the test for ivy.inplace_increment should give you an idea of how to test this frontend function because it is kind of similar to what's happening with ivy.inplace_increment I would think so. Although there are plenty of functions in our frontends that perform updates inplace (functions with names ending at an underscore "_"), so not sure why we need some special handling over here?

ivy_tests/test_ivy/test_frontends/test_paddle/test_tensor/test_math.py

ivy/functional/frontends/paddle/tensor/math.py

hmahmood24 · 2023-09-03T16:46:12Z

ivy_tests/test_ivy/test_frontends/test_paddle/test_tensor/test_math.py

@@ -1864,3 +1864,24 @@ def test_paddle_diff(
        prepend=prepend[0],
        append=append[0],
    )
+
+


@FelixHuyghe Try copying the array using something like ivy.copy_array before passing it to ivy.inplace_increment. See if that works for you and let me know :)

ivy_tests/test_ivy/test_frontends/test_paddle/test_tensor/test_math.py

hmahmood24 · 2023-09-03T16:54:28Z

ivy/functional/frontends/paddle/tensor/math.py

+)
+@to_ivy_arrays_and_back
+def increment(x, value=1.0):
+    if x.size == 1:


@FelixHuyghe Why is this if condition needed anyways?

FelixHuyghe · 2023-09-07T11:13:07Z

Hey, I got the tests (partially) working! I do however have two main remaining problems. I left comments on your old comments, so I think you will have to scroll up quite a bit to get to them :)

hmahmood24

Hey @FelixHuyghe, left some replies to your comments. Let me know if they help?

hmahmood24 · 2023-10-15T15:56:26Z

Closing this PR since it has been stale for more than 2 weeks now.

add increment method

ff8f793

Update test_math.py fix casing

abfd76b

FelixHuyghe changed the title ~~add increment method~~ add increment method #21294 Aug 6, 2023

FelixHuyghe changed the title ~~add increment method #21294~~ add increment method Aug 6, 2023

FelixHuyghe changed the title ~~add increment method~~ Add increment method to Paddle frontend Aug 6, 2023

ivy-leaves added the Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist label Aug 6, 2023

ivy-seed assigned hmahmood24 Aug 6, 2023

hmahmood24 requested changes Aug 13, 2023

View reviewed changes

add testing, test not working

40e81a4

FelixHuyghe requested a review from hmahmood24 August 30, 2023 15:39

hmahmood24 requested changes Sep 3, 2023

View reviewed changes

add copy and datatype matching

89ae691

FelixHuyghe requested a review from hmahmood24 September 7, 2023 11:08

hmahmood24 requested changes Sep 11, 2023

View reviewed changes

juliagsy mentioned this pull request Oct 5, 2023

feat: add increment method to paddle frontend #26331

Closed

5 tasks

hmahmood24 closed this Oct 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add increment method to Paddle frontend #21369

Add increment method to Paddle frontend #21369

FelixHuyghe commented Aug 6, 2023 •

edited

Loading

github-actions bot commented Aug 6, 2023

hmahmood24 left a comment

hmahmood24 Aug 13, 2023

FelixHuyghe Aug 30, 2023 •

edited

Loading

hmahmood24 Sep 3, 2023

FelixHuyghe Sep 7, 2023 •

edited

Loading

hmahmood24 Sep 11, 2023

hmahmood24 Aug 13, 2023

FelixHuyghe Aug 30, 2023 •

edited

Loading

hmahmood24 Sep 3, 2023

FelixHuyghe Sep 7, 2023 •

edited

Loading

hmahmood24 Sep 11, 2023 •

edited

Loading

hmahmood24 Sep 3, 2023

hmahmood24 Sep 3, 2023

FelixHuyghe commented Sep 7, 2023 •

edited

Loading

hmahmood24 left a comment

hmahmood24 commented Oct 15, 2023

Add increment method to Paddle frontend #21369

Add increment method to Paddle frontend #21369

Conversation

FelixHuyghe commented Aug 6, 2023 • edited Loading

github-actions bot commented Aug 6, 2023

hmahmood24 left a comment

Choose a reason for hiding this comment

hmahmood24 Aug 13, 2023

Choose a reason for hiding this comment

FelixHuyghe Aug 30, 2023 • edited Loading

Choose a reason for hiding this comment

hmahmood24 Sep 3, 2023

Choose a reason for hiding this comment

FelixHuyghe Sep 7, 2023 • edited Loading

Choose a reason for hiding this comment

hmahmood24 Sep 11, 2023

Choose a reason for hiding this comment

hmahmood24 Aug 13, 2023

Choose a reason for hiding this comment

FelixHuyghe Aug 30, 2023 • edited Loading

Choose a reason for hiding this comment

hmahmood24 Sep 3, 2023

Choose a reason for hiding this comment

FelixHuyghe Sep 7, 2023 • edited Loading

Choose a reason for hiding this comment

hmahmood24 Sep 11, 2023 • edited Loading

Choose a reason for hiding this comment

hmahmood24 Sep 3, 2023

Choose a reason for hiding this comment

hmahmood24 Sep 3, 2023

Choose a reason for hiding this comment

FelixHuyghe commented Sep 7, 2023 • edited Loading

hmahmood24 left a comment

Choose a reason for hiding this comment

hmahmood24 commented Oct 15, 2023

FelixHuyghe commented Aug 6, 2023 •

edited

Loading

FelixHuyghe Aug 30, 2023 •

edited

Loading

FelixHuyghe Sep 7, 2023 •

edited

Loading

FelixHuyghe Aug 30, 2023 •

edited

Loading

FelixHuyghe Sep 7, 2023 •

edited

Loading

hmahmood24 Sep 11, 2023 •

edited

Loading

FelixHuyghe commented Sep 7, 2023 •

edited

Loading