feat(numpy): add signbit #23333

RohitSgh · 2023-09-09T16:43:50Z

PR Description

Adding signbit
https://numpy.org/doc/stable/reference/generated/numpy.signbit.html

Related Issue

Close #23332

Checklist

Did you add a function?
Did you add the tests?
Did you follow the steps we provided?

Socials:

https://x.com/iRohitSgh
https://www.linkedin.com/in/iRohitSgh

github-actions · 2023-09-09T16:44:06Z

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

RohitSgh · 2023-09-09T17:17:49Z

Hi @KareemMAX. One test intelligent-tests-pr / run_tests (1) (pull_request) is in progress for long. Any idea on how to proceed?

Edit: PR failed after 72 minutes

RohitSgh · 2023-09-09T18:16:21Z

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.

Please note that the contributor is not expected to understand everything in the checklist. It's mainly here for the reviewer to make sure everything has been done correctly 🙂

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. - x] 🆗: 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 · 2023-09-11T08:42:29Z

Hi @KareemMAX. One test intelligent-tests-pr / run_tests (1) (pull_request) is in progress for long. Any idea on how to proceed?

Edit: PR failed after 72 minutes

No worries @RohitSgh, as the automatic comment has mentioned, feel free to ignore those tests.

KareemMAX

Hey @RohitSgh,

Great work, I've pointed out a small detail that I think need to be made before we proceed with the final tests. Also if you can change the dtypes from float to valid. Otherwise LGTM 🚀 .

Thanks

ivy/functional/frontends/numpy/mathematical_functions/floating_point_routines.py

RohitSgh · 2023-09-11T09:26:31Z

Thanks for the review @KareemMAX. By the way, is the implementation memory efficient?

It uses numpy.less (ivy.less), for purpose of which we create another array of same size as input x

KareemMAX · 2023-09-11T09:42:14Z

ivy/functional/frontends/numpy/mathematical_functions/floating_point_routines.py

+    if dtype is None:
+        dtype = ivy.dtype(x)
+    zero = ivy.zeros_like(x, dtype=dtype)
+    return ivy.less(x, zero)


My bad 😅, I forgot to mention that as will.

Suggested change

if dtype is None:

dtype = ivy.dtype(x)

zero = ivy.zeros_like(x, dtype=dtype)

return ivy.less(x, zero)

return x < 0

I guess that would be a more efficient solution. I removed dtype as it's supposed to be handled by ufunc.

Also I'm not sure a bit about this implementation, as I've seen a case where the sign bit is toggled with the value 0 (-0). Can you research about how this might impact this implementation? I believe it might

These are my findings when I tried np.signbit on my console

>>> numpy.signbit(-0.0) True >>> numpy.signbit(0.0) False

And

>>> numpy.signbit(-0) False

Actually it was summarized here as well https://note.nkmk.me/en/python-numpy-sign-signbit-copysign/

github-actions

PR Compliance Checks Passed!

RohitSgh · 2023-09-13T22:34:21Z

Tried to handle the situation by using https://numpy.org/doc/stable/reference/routines.logic.html and https://numpy.org/doc/stable/reference/generated/numpy.arctan2.html, @KareemMAX

RohitSgh · 2023-09-18T22:57:15Z

Hi @KareemMAX. Can you please review the changes? Thanks!

KareemMAX

Hey @RohitSgh,
I've tried to add some fixes to your code. But we need to merge to main to ensure that you are not changing the demos submodule which looks like you have edited by mistake.

Also tests are failing, please look at them.

RohitSgh · 2023-09-22T23:30:18Z

Hi @KareemMAX. The failing PR are of type run_tests. It was supposed to be ignored. Isn't it?

KareemMAX · 2023-09-28T11:04:49Z

Hi @KareemMAX. The failing PR are of type run_tests. It was supposed to be ignored. Isn't it?

Yes @RohitSgh, you should ignore run_tests. You may use this command to test your code locally:

pytest -v ivy_tests/test_ivy/test_frontends/test_numpy/test_mathematical_functions/test_floating_point_routines.py::test_numpy_signbit

Also, demos seems to still be conflicting 😅

RohitSgh · 2023-09-29T09:51:02Z

The test couldn't find arctan2. However, #23315 was recently merged.

Any idea on how to proceed, @KareemMAX ?

Link
https://unify.ai/docs/ivy/overview/contributing/open_tasks.html#frontend-apis

KareemMAX · 2023-09-29T12:16:28Z

Any idea on how to proceed, @KareemMAX ?

@RohitSgh, the mentioned PR does add a numpy frontend function. You are trying to call an ivy function that doesn't exist

RohitSgh · 2023-09-29T12:46:10Z

@KareemMAX

But my implementation wishes to use arctan2 as mentioned here.

arctan2 is mentioned in Add Mathematical functions to Numpy Frontend #1525 as well.

Is there any way by which I can use a proposed function?
I don't know why demos are producing conflict. Is there any "Back to Square One" approach which I can use for the same?

KareemMAX · 2023-09-29T13:17:13Z

But my implementation wishes to use arctan2 as mentioned here.

This is numpy frontend, not an ivy function. ivy.archtan2 doesn't exist

I don't know why demos are producing conflict. Is there any "Back to Square One" approach which I can use for the same?

You can use git reset --hard <commit-hash> and force push to your branch

RohitSgh · 2023-09-30T03:53:48Z

This is numpy frontend, not an ivy function. ivy.archtan2 doesn't exist

But it is indeed an proposed function which is listed in #1525. Our signbit is also listed in same #1525. Am I missing anything?

Also, https://unify.ai/docs/ivy/overview/contributing/open_tasks.html#frontend-apis suggests that we can use composition of proposed function as well.

Another query, if ivy.arctan2 doesn't exist, can I use numpy.arctan2 in implementation of signbit?

Regarding docs/demos, can I assume no conflict based on this screenshot?

KareemMAX · 2023-10-02T12:17:21Z

But it is indeed an proposed function which is listed in #1525. Our signbit is also listed in same #1525. Am I missing anything?

As I mentioned before, this list is for frontend functions. There is no ivy.arctan2.

Another query, if ivy.arctan2 doesn't exist, can I use numpy.arctan2 in implementation of signbit?

Yes, I guess you can use numpy_frontend variable. You will find lots of people using it through out our numpy frontend code

RohitSgh · 2023-10-02T15:02:19Z

@KareemMAX

Earlier, the error were

Cannot cast ufunc 'signbit' input from dtype('int8') to dtype('float16') with casting rule 'no'

Added two assume statement. Now no error on test.

Is it fine?

KareemMAX

Hey @RohitSgh, I requested the help of our team internally to know if you are doing the right thing. Here is what @AnnaTz thinks:

You shouldn't use assume(casting == "same_kind"), neither assume(dtype != "bool"). The latter should be avoided by adding @with_unsupported_dtypes to the function. About the casting, your function implementation should be able to handle all casting modes. There are more arguments that are not being handled either, only x is being used currently.

Also it seems like you removed docs/demos, we can't merge your PR unless you revert back that change

github-actions · 2023-10-09T20:44:29Z

Thank you for this PR, here is the CI results:

This pull request does not result in any additional test failures. Congratulations!

RohitSgh · 2023-10-25T08:32:15Z

Completed in #26845

ivy-leaves added the NumPy Frontend Developing the NumPy Frontend, checklist triggered by commenting add_frontend_checklist label Sep 9, 2023

ivy-seed assigned KareemMAX Sep 9, 2023

KareemMAX suggested changes Sep 11, 2023

View reviewed changes

ivy/functional/frontends/numpy/mathematical_functions/floating_point_routines.py Outdated Show resolved Hide resolved

KareemMAX reviewed Sep 11, 2023

View reviewed changes

github-actions bot reviewed Sep 13, 2023

View reviewed changes

RohitSgh requested a review from KareemMAX September 13, 2023 22:33

RohitSgh changed the title ~~signbit~~ feat(numpy): add signbit Sep 13, 2023

KareemMAX suggested changes Sep 21, 2023

View reviewed changes

RohitSgh requested a review from KareemMAX October 2, 2023 14:53

KareemMAX suggested changes Oct 4, 2023

View reviewed changes

RohitSgh closed this Oct 9, 2023

RohitSgh deleted the signbit branch October 9, 2023 20:43

RohitSgh restored the signbit branch October 9, 2023 20:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(numpy): add signbit #23333

feat(numpy): add signbit #23333

RohitSgh commented Sep 9, 2023 •

edited

Loading

github-actions bot commented Sep 9, 2023

RohitSgh commented Sep 9, 2023 •

edited

Loading

RohitSgh commented Sep 9, 2023 •

edited by KareemMAX

Loading

KareemMAX commented Sep 11, 2023

KareemMAX left a comment

RohitSgh commented Sep 11, 2023

KareemMAX Sep 11, 2023

KareemMAX Sep 11, 2023

RohitSgh Sep 13, 2023

github-actions bot left a comment •

edited

Loading

RohitSgh commented Sep 13, 2023

RohitSgh commented Sep 18, 2023

KareemMAX left a comment

RohitSgh commented Sep 22, 2023

KareemMAX commented Sep 28, 2023

RohitSgh commented Sep 29, 2023 •

edited

Loading

KareemMAX commented Sep 29, 2023

RohitSgh commented Sep 29, 2023 •

edited

Loading

KareemMAX commented Sep 29, 2023

RohitSgh commented Sep 30, 2023

KareemMAX commented Oct 2, 2023

RohitSgh commented Oct 2, 2023 •

edited

Loading

KareemMAX left a comment

github-actions bot commented Oct 9, 2023

RohitSgh commented Oct 25, 2023

feat(numpy): add signbit #23333

feat(numpy): add signbit #23333

Conversation

RohitSgh commented Sep 9, 2023 • edited Loading

PR Description

Related Issue

Checklist

Socials:

github-actions bot commented Sep 9, 2023

RohitSgh commented Sep 9, 2023 • edited Loading

RohitSgh commented Sep 9, 2023 • edited by KareemMAX Loading

Frontend Task Checklist

IMPORTANT NOTICE 🚨:

LEGEND 🗺:

CHECKS 📑:

KareemMAX commented Sep 11, 2023

KareemMAX left a comment

Choose a reason for hiding this comment

RohitSgh commented Sep 11, 2023

KareemMAX Sep 11, 2023

Choose a reason for hiding this comment

KareemMAX Sep 11, 2023

Choose a reason for hiding this comment

RohitSgh Sep 13, 2023

Choose a reason for hiding this comment

github-actions bot left a comment • edited Loading

Choose a reason for hiding this comment

RohitSgh commented Sep 13, 2023

RohitSgh commented Sep 18, 2023

KareemMAX left a comment

Choose a reason for hiding this comment

RohitSgh commented Sep 22, 2023

KareemMAX commented Sep 28, 2023

RohitSgh commented Sep 29, 2023 • edited Loading

KareemMAX commented Sep 29, 2023

RohitSgh commented Sep 29, 2023 • edited Loading

KareemMAX commented Sep 29, 2023

RohitSgh commented Sep 30, 2023

KareemMAX commented Oct 2, 2023

RohitSgh commented Oct 2, 2023 • edited Loading

KareemMAX left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 9, 2023

RohitSgh commented Oct 25, 2023

RohitSgh commented Sep 9, 2023 •

edited

Loading

RohitSgh commented Sep 9, 2023 •

edited

Loading

RohitSgh commented Sep 9, 2023 •

edited by KareemMAX

Loading

github-actions bot left a comment •

edited

Loading

RohitSgh commented Sep 29, 2023 •

edited

Loading

RohitSgh commented Sep 29, 2023 •

edited

Loading

RohitSgh commented Oct 2, 2023 •

edited

Loading