Skip to content

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

3 participants