Modularize NotEqual. #3919
Modularize NotEqual. #3919
Conversation
| const dataId = cpuBackend.write(resultData, resultShape, a.dtype); | ||
| return {dataId, shape: resultShape, dtype: a.dtype}; | ||
| } | ||
| export type SimpleBinaryOperation = (a: number, b: number) => number; |
There was a problem hiding this comment.
nit: might be useful to use this type to replace the type of the "op" parameter in createBinaryKernelImpl below.
lina128
left a comment
lina128
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 1 approvals obtained (waiting on @annxingyuan, @jinjingforever, and @tafsiri)
tfjs-backend-cpu/src/utils/kernel_utils.ts, line 26 at r1 (raw file):
Previously, jinjingforever (Jing Jin) wrote…
nit: might be useful to use this type to replace the type of the "op" parameter in createBinaryKernelImpl below.
Ah, good catch. Done.
tafsiri
left a comment
tafsiri
left a comment
There was a problem hiding this comment.
LGTM with one suggestion.
Reviewed 4 of 5 files at r1, 1 of 1 files at r2.
Reviewable status:
tfjs-backend-cpu/src/kernels/NotEqual.ts, line 22 at r2 (raw file):
createBinaryKernelConfig
More of a design/dx question. Could you pass the anonymous function directly to binaryKernelFunc below and have it call createBinaryKernelImpl internally? It seems like it is always called the same way by each caller so might as well be included in binaryKernelFunc (this would also affect your other pr with complex op examples). There could be a use case I am missing though.
annxingyuan
left a comment
annxingyuan
left a comment
There was a problem hiding this comment.
LGTM
Reviewable status:
lina128
left a comment
lina128
left a comment
There was a problem hiding this comment.
Reviewable status:
tfjs-backend-cpu/src/kernels/NotEqual.ts, line 22 at r2 (raw file):
Previously, tafsiri (Yannick Assogba) wrote…
createBinaryKernelConfigMore of a design/dx question. Could you pass the anonymous function directly to binaryKernelFunc below and have it call createBinaryKernelImpl internally? It seems like it is always called the same way by each caller so might as well be included in binaryKernelFunc (this would also affect your other pr with complex op examples). There could be a use case I am missing though.
Ah, good idea. Refactored.
5 participants
Modularized NotEqual and refactored the kernel utils. This PR has below changes:
This change is