Implement element-wise logical operations#55
Implement element-wise logical operations#55huningxin merged 3 commits intowebmachinelearning:mainfrom
Conversation
equal / greater / greaterOrEqual
lesser / lesserOrEqual / not
src/logical.js
Outdated
| * @param {Function} logicalFunc | ||
| * @return {Tensor} | ||
| */ | ||
| function logical(inputA, inputB, logicalFunc) { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Updated. Please take another look, thanks.
fdwr
left a comment
There was a problem hiding this comment.
Only minor readibility comments. Thanks Bruce.
src/lib/validate-input.js
Outdated
| for (let i = 0; i < sizeOfShape(input.shape); ++i) { | ||
| const a = input.getValueByIndex(i); | ||
| if (!Number.isInteger(a)) { | ||
| throw new Error('Invalid input value, it should be an integer in the interval [0, 255]'); |
There was a problem hiding this comment.
| throw new Error('Invalid input value, it should be an integer in the interval [0, 255]'); | |
| throw new Error('Invalid input value - it should be an integer in the interval [0, 255]'); |
or
- throw new Error('Invalid input value, it should be an integer in the interval [0, 255]');
+ throw new Error('Invalid input value: it should be an integer in the interval [0, 255]');or
- throw new Error('Invalid input value, it should be an integer in the interval [0, 255]');
+ throw new Error('Invalid input value. It should be an integer in the interval [0, 255]');(minor grammar) An em dash or colon would be more appropriate here, as they set off extra information such as explanatory phrases or supplemental facts in a later clause that explains or expands upon on what precedes it. Commas are more for dependent clauses.
There was a problem hiding this comment.
Btw, did you intend to use a distinct error message for these two cases? Otherwise we could collapse 7 lines to 3 lines:
if (!Number.isInteger(a) || a < 0 || a > 255) {
throw new Error('Invalid input value - it should be an integer in the interval [0, 255]');
}
src/logical.js
Outdated
| export const greaterOrEqual = (inputA, inputB) => logical(inputA, inputB, (a, b) => a >= b); | ||
| export const lesser = (inputA, inputB) => logical(inputA, inputB, (a, b) => a < b); | ||
| export const lesserOrEqual = (inputA, inputB) => logical(inputA, inputB, (a, b) => a <= b); |
There was a problem hiding this comment.
👀🔍 It's visually a little challenging to parse these two parts:
(a, b) => a >= b
(a, b) => a <= b
Considering wrapping them for increased readability:
(a, b) => (a >= b)
(a, b) => (a <= b)
bb28320 to
8d97a25
Compare
@huningxin @fdwr PTAL, thanks.