EHN: adding bitwise funtion in paddle tensor.py #17416

MuhammadNizamani · 2023-06-20T13:15:32Z

Adding bitwise_xor, Closes #17414

KareemMAX · 2023-06-20T19:28:24Z

Frontend Task Checklist

IMPORTANT NOTICE 🚨:

The Ivy Docs represent the ground truth for the task descriptions and this checklist should only be used as a supplementary item to aid with the review process.

LEGEND 🗺:

❌ : Check item is not completed.
✅ : Check item is ready for review.
🆘 : Stuck/Doubting implementation (PR author should add comments explaining why).
⏩ : Check is not applicable to function (skip).
🆗 : Check item is implemented and does not require any edits.

CHECKS 📑:

- 🆗: The function/method definition is not missing any of the original arguments.
- ⏩: In case the function/method to be implemented is an alias of an existing function/method:
  1. - ⏩: It is being declared as such by setting fun1 = fun2, rather than being re-implemented from scratch.
  2. - ⏩: The alias is added to the existing function/method's test in the aliases parameter of handle_frontend_test/handle_frontend_method.
- 🆗: The naming of the function/method and its arguments exactly matches the original.
- 🆗: No defined argument is being ignored in the function/method's implementation.
- ⏩: In special cases where an argument's implementation should be pending due to an incomplete superset of an ivy function:
  1. - ⏩: A descriptive comment has been left under the Implement superset behavior ToDo list in Implement superset behavior #6406.
  2. - ⏩: A ToDo comment has been added prompting to pass the frontend argument to the ivy function whose behavior is to be extended.
- ⏩: In case a frontend function is being added:
  1. - ⏩: It is a composition of ivy functions.
  2. - ⏩: In case the needed composition is long (using numerous ivy functions), a Missing Function Suggestion issue has been opened to suggest a new ivy function should be added to shorten the frontend implementation.
  3. - ⏩: @to_ivy_arrays_and_back has been added to the function.
- 🆗: In case a frontend method is being added:
  1. - 🆗: It is composed of existing frontend functions or methods.
  2. - 🆗: If a required frontend function has not yet been added, the method may be implemented as a composition of ivy functions, making sure that:
      - 🆗: @to_ivy_arrays_and_back has been added to the method.
      - 🆗: A ToDo comment has been made prompting to remove the decorator and update the implementation as soon as the missing function has been added.
- 🆗: The function/method's test has been added (except in the alias case mentioned in <2>):
  1. - 🆗: All supported arguments are being generated in handle_frontend_test/handle_frontend_method and passed to test_frontend_function/test_frontend_method.
  2. - 🆗: The argument generation covers all possible supported values. Array sizes, dimensions, and axes adhere to the full supported set of the original function/method.
  3. - 🆗: The available_dtypes parameter passed to the helper generating the function/method's input array is set to helpers.get_dtypes("valid"). If there are unsupported dtypes that cause the test to fail, they should be handled by adding @with_supported_dtypes/@with_unsupported_dtype to the function/method.
- 🆗: The PR is not introducing any test failures.
  1. - 🆗: The lint checks are passing.
  2. - 🆗: The implemented test is passing for all backends.
- 🆗: The PR closes a Sub Task issue linked to one of the open frontend ToDo lists.
- 🆗: The function/method and its test have been added to the correct .py files corresponding to the addressed ToDo list.
- 🆗: The PR only contains changes relevant to the addressed subtask.

KareemMAX

Great job @MuhammadNizamani 🚀🚀

I've made a small comment, maybe I might be mistaken about the to_ivy_and_back thing so I'm also asking if there is any internal changes that I'm not aware about. Please look at my comment I'd like to hear your thoughts

KareemMAX · 2023-06-20T19:32:16Z

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

+        "paddle",
+    )
+    def bitwise_xor(self, y, out=None, name=None):
+        y_ivy = _to_ivy_array(y)


Is there is a specific reason why we use _to_ivy_array? I remember that we used @to_ivy_arrays_and_back() for this conversion but I can't find it in the rest of the file so I'm checking why it isn't there

@KareemMAX we are implementing method in Tensor class here and we take our first input using self and by using self where can convert our input into ivy array so no need of @to_ivy_arrays_and_back().
I am using _to_ivy_array because if i use @to_ivy_arrays_and_back() then it will became redundant with this self._ivy_array

Yup, I got mixed up here

But I'm still curious that in other functions it's calling paddle_frontend like in tensor.multiply() https://github.com/unifyai/ivy/pull/17416/files#diff-4b9f6f2107068e5624a6f5d66bae5dae58c467a34f8c5aad65df3478ce4a5abbR187

@with_unsupported_dtypes({"2.4.2 and below": ("float16", "bfloat16")}, "paddle") def multiply(self, y, name=None): return paddle_frontend.multiply(self, y)

The implementation appears to be incorrect based on my understanding. According to my understanding, paddle.tensor.Tensor should receive functions from the backend, rather than the frontend. However, the above function is using paddle.tensor.tensor.math.py to access the backend.

paddle_frontend is the ivy implementation of paddle frontend. We are not calling any paddle functions if that is what do you mean.

I've asked @hmahmood24 and he said the following:

since instance methods operate on the instance of the frontend class (paddle.Tensor in this case) which have a _ivy_array attribute that holds the ivy.Array already) and there are only 2 ways we go about writing these instance methods:

Calling ivy.<func> directly
Calling a frontend function of the same functionality paddle_frontend.<func>

That's why I think this implementation is wrong and multiply() is have the correct implementation

@KareemMAX he mentioned that you can also implement like ivy.func so I used that implemention .

@KareemMAX done

KareemMAX · 2023-06-22T11:50:29Z

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

+        "paddle",
+    )
+    def bitwise_xor(self, y, out=None, name=None):
+        y_ivy = _to_ivy_array(y)


Yup, I got mixed up here

But I'm still curious that in other functions it's calling paddle_frontend like in tensor.multiply() https://github.com/unifyai/ivy/pull/17416/files#diff-4b9f6f2107068e5624a6f5d66bae5dae58c467a34f8c5aad65df3478ce4a5abbR187

MuhammadNizamani · 2023-06-22T18:45:07Z

@KareemMAX sir, when you are going to merge this PR ?

KareemMAX · 2023-06-22T19:54:40Z

@MuhammadNizamani once we fix the current issue your PR will be good to merge after running tests. Please refer to the latest comment I made on the unresolved conversation

KareemMAX · 2023-06-23T11:32:59Z

LGTM! merging now

Thanks for your work @MuhammadNizamani 🚀🚀🚀

Co-authored-by: KareemMAX <kreem.morsy@hotmail.com>

EHN: adding bitwise funtion in paddle tensor.py

dee912f

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

ivy-seed assigned KareemMAX Jun 20, 2023

ivy-llc deleted a comment from ivy-leaves Jun 20, 2023

KareemMAX suggested changes Jun 20, 2023

View reviewed changes

MuhammadNizamani added 3 commits June 21, 2023 08:15

Merge branch 'master' into bitwise_xor

1c29a6f

Merge branch 'master' into bitwise_xor

2f91bf4

Merge branch 'master' into bitwise_xor

98923a9

KareemMAX suggested changes Jun 22, 2023

View reviewed changes

Merge branch 'master' into bitwise_xor

dd12b7f

MuhammadNizamani added 3 commits June 23, 2023 10:11

calling function from frontend

8724a6c

Merge branch 'master' into bitwise_xor

80d103f

Merge branch 'master' into bitwise_xor

f2afe86

KareemMAX approved these changes Jun 23, 2023

View reviewed changes

KareemMAX merged commit c1906ec into ivy-llc:master Jun 23, 2023

MuhammadNizamani deleted the bitwise_xor branch June 24, 2023 10:46

Mghrabi pushed a commit to Mghrabi/ivy that referenced this pull request Jun 24, 2023

EHN: adding bitwise funtion in paddle tensor.py (ivy-llc#17416)

96ff943

Co-authored-by: KareemMAX <kreem.morsy@hotmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EHN: adding bitwise funtion in paddle tensor.py #17416

EHN: adding bitwise funtion in paddle tensor.py #17416

MuhammadNizamani commented Jun 20, 2023 •

edited by KareemMAX

Loading

KareemMAX commented Jun 20, 2023 •

edited

Loading

KareemMAX left a comment

KareemMAX Jun 20, 2023

MuhammadNizamani Jun 21, 2023

KareemMAX Jun 22, 2023

MuhammadNizamani Jun 22, 2023

KareemMAX Jun 22, 2023

MuhammadNizamani Jun 22, 2023 •

edited

Loading

MuhammadNizamani Jun 23, 2023

KareemMAX Jun 22, 2023

MuhammadNizamani commented Jun 22, 2023

KareemMAX commented Jun 22, 2023

KareemMAX commented Jun 23, 2023

EHN: adding bitwise funtion in paddle tensor.py #17416

EHN: adding bitwise funtion in paddle tensor.py #17416

Conversation

MuhammadNizamani commented Jun 20, 2023 • edited by KareemMAX Loading

KareemMAX commented Jun 20, 2023 • edited Loading

Frontend Task Checklist

IMPORTANT NOTICE 🚨:

LEGEND 🗺:

CHECKS 📑:

KareemMAX left a comment

Choose a reason for hiding this comment

KareemMAX Jun 20, 2023

Choose a reason for hiding this comment

MuhammadNizamani Jun 21, 2023

Choose a reason for hiding this comment

KareemMAX Jun 22, 2023

Choose a reason for hiding this comment

MuhammadNizamani Jun 22, 2023

Choose a reason for hiding this comment

KareemMAX Jun 22, 2023

Choose a reason for hiding this comment

MuhammadNizamani Jun 22, 2023 • edited Loading

Choose a reason for hiding this comment

MuhammadNizamani Jun 23, 2023

Choose a reason for hiding this comment

KareemMAX Jun 22, 2023

Choose a reason for hiding this comment

MuhammadNizamani commented Jun 22, 2023

KareemMAX commented Jun 22, 2023

KareemMAX commented Jun 23, 2023

MuhammadNizamani commented Jun 20, 2023 •

edited by KareemMAX

Loading

KareemMAX commented Jun 20, 2023 •

edited

Loading

MuhammadNizamani Jun 22, 2023 •

edited

Loading