Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add bitwiseAnd() ops #7645

Merged
merged 16 commits into from
May 3, 2023
Merged

Add bitwiseAnd() ops #7645

merged 16 commits into from
May 3, 2023

Conversation

fengwuyao
Copy link
Collaborator

  • Implemented the bitwiseAnd() ops
  • Added the kernel in CPU backend
  • Example usage:
const x = tf.tensor1d([0, 5, 3, 14]);
const y = tf.tensor1d([5, 0, 7, 11]);
tf.bitwiseAnd(x, y).print();

mattsoulanille
mattsoulanille requested changes May 2, 2023
View reviewed changes
tfjs-backend-cpu/src/kernels/BitwiseAnd.ts Outdated Show resolved Hide resolved
tfjs-backend-cpu/src/kernels/BitwiseAnd.ts Outdated Show resolved Hide resolved
tfjs-backend-cpu/src/register_all_kernels.ts Outdated Show resolved Hide resolved
tfjs-backend-cpu/src/shared.ts Outdated Show resolved Hide resolved
tfjs-converter/src/operations/executors/logical_executor.ts Outdated Show resolved Hide resolved
tfjs-core/src/ops/bitwise_and_test.ts Outdated Show resolved Hide resolved
tfjs-core/src/ops/bitwise_and.ts Outdated Show resolved Hide resolved
@fengwuyao fengwuyao force-pushed the addOps branch 2 times, most recently from a13ef70 to b30f498 Compare May 2, 2023 19:42
mattsoulanille
mattsoulanille approved these changes May 2, 2023
View reviewed changes
Copy link
Member

@mattsoulanille mattsoulanille left a comment

Choose a reason for hiding this comment

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

LGTM!

tfjs-converter/src/operations/executors/transformation_executor_test.ts Outdated Show resolved Hide resolved
tfjs-core/src/ops/bitwise_and_test.ts Outdated Show resolved Hide resolved
.vscode/settings.json Show resolved Hide resolved
Linchenn
Linchenn reviewed May 2, 2023
View reviewed changes
tfjs-backend-cpu/src/shared.ts
@@ -19,6 +19,7 @@
export {simpleAbsImpl} from './kernels/Abs';
export {addImpl} from './kernels/Add';
export {bincountImpl, bincountReduceImpl} from './kernels/Bincount_impl';
export {bitwiseAndImpl} from './kernels/BitwiseAnd';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice catch. Adding it here means other backend could use this kernel implementation. We could discuss this later when implementing this kernel for other backends.

tfjs-core/src/ops/bitwise_and.ts Outdated
throw new Error(`BitwiseAnd: Tensors must have the same shape. x: ${
$x.shape}, y: ${$y.shape}`);
}
if ($x.dtype !== 'float32' || $y.dtype !== 'float32') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should it be 'int32'?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for pointing out. Updated.

tfjs-core/src/ops/bitwise_and.ts Outdated Show resolved Hide resolved
@fengwuyao fengwuyao requested a review from Linchenn May 2, 2023 23:48
Linchenn
Linchenn approved these changes May 2, 2023
View reviewed changes
Copy link
Collaborator

@Linchenn Linchenn left a comment

Choose a reason for hiding this comment

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

Just a nit. LGTM, this is very fast! Thanks!

tfjs-core/src/ops/bitwise_and.ts Outdated Show resolved Hide resolved
@fengwuyao fengwuyao merged commit 2863235 into tensorflow:master May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Linchenn Linchenn Linchenn approved these changes

@mattsoulanille mattsoulanille mattsoulanille approved these changes

@chunnienc chunnienc Awaiting requested review from chunnienc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fengwuyao @mattsoulanille @Linchenn