Skip to content

Add bitwise elementwise specifications #54

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

Merged
merged 3 commits into from
Oct 26, 2020
Merged

Add bitwise elementwise specifications #54

merged 3 commits into from
Oct 26, 2020

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Oct 19, 2020

This PR

  • adds bitwise elementwise functional specification equivalents for the following operators: &, |, ^, ~, >>, and <<.
  • is derived from comparing API signatures across array libraries.

Notes

  • The specifications follow the same form as other elementwise functions.

  • Functional bitwise APIs are, while not ubiquitous, commonly implemented across array libraries. The principle impetus for their inclusion is to have functional equivalents of operators (as discussed and endorsed during a prior consortium meeting).

  • This PR induces a slightly different naming convention. NumPy (and its followers) splits its namespace into non-prefixed and prefixed APIs (e.g., right_shift vs bitwise_and). TensorFlow puts everything under a tf.bitwise namespace, while also using a bitwise_ prefix for certain APIs.

    This PR chooses to simply put all bitwise operations under a bitwise_ prefix. This matches, and is consistent with, the current convention of using a logical_ prefix for logical operations.

  • This PR currently requires that x2 for both right_shift and left_shift have nonnegative elements. NumPy's docs for left_shift state that this is required; although, from the interpreter, no error is thrown if a negative x2 is provided. TensorFlow states that negative x2 results in implementation-dependent behavior.

    This PR takes the stance that negative x2 for either left_shift and right_shift is probably a mistake and should not be permitted. Hence, the included requirements that x2 be greater than or equal to 0.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @kgryte. I like the choices made and the consistency of the resulting API here. Since this deviates a little from existing implementations, I'll leave it open for at least a few days in case there are more comments.

@kgryte
Copy link
Contributor Author

kgryte commented Oct 26, 2020

@rgommers I've updated the bitwise_left_shift and bitwise_right_shift function names per the discussion in the previous consortium meeting. This PR should be ready for merge.

@rgommers rgommers merged commit 5216142 into master Oct 26, 2020
@rgommers rgommers deleted the bitwise branch October 26, 2020 19:34
@rgommers
Copy link
Member

Thanks @kgryte, merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants