Completed constant #65
Conversation
test/constant_test.js
Outdated
| describe('test constant', function() { | ||
| function testConstant(start, step, outputShape, type, expected) { | ||
| const outputTensor = constant(start, step, outputShape, type); | ||
| console.log('outputTensor', outputTensor); |
There was a problem hiding this comment.
Please remove these two console.log codes.
src/constant.js
Outdated
| resultArray.push(start + i * step); | ||
| } | ||
| const resultToTensor = new Tensor([resultArray.length], resultArray); | ||
| const transformTensorType = cast(resultToTensor, type); |
There was a problem hiding this comment.
Since we did not make corresponding checks or specifications for data out of bounds in the #63 cast function, should we clarify the behavior of the cases here?
- constant(start = 127, step = 1, outputShape = [3], type = 'int8');
- constant(start = -10, step = 1, outputShape = [3, 3], type = 'uint32');
- constant(start = 10, step = -2, outputShape = [3, 3], type = 'uint32');
......
There was a problem hiding this comment.
Different hardware will behave differently (CPU, NPU, GPU), so I think these cases may be unnecessary.
mei1127
force-pushed
the
add_constant
branch
from
d91574c
to
a4d684c
Compare
src/constant.js
Outdated
| for (let i = 0; i < outputElementCount; i++) { | ||
| resultArray.push(start + i * step); | ||
| } | ||
| const resultToTensor = new Tensor([resultArray.length], resultArray); |
There was a problem hiding this comment.
Just create the tensor with the output shape upfront?
const resultToTensor = new Tensor(outputShape, resultArray);
const output = cast(resultToTensor, type);
return output;
| }; | ||
| testConstant( | ||
| 1, 0, [3, 3], 'float64', expected); | ||
| }); |
There was a problem hiding this comment.
Would be good to include one scalar sanity-check case. e.g.
it('constant scalar', function() {
const expected ={
shape: [],
data: [42],
};
testConstant(42, 0, [], 'float64', expected);
| import {cast} from './cast.js'; | ||
| import {Tensor, sizeOfShape} from '../src/lib/tensor.js'; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Please update this function's description and also add descriptions for start, step and type parameters.
src/constant.js
Outdated
|
|
||
| export function constant(start, step, outputShape, type = 'float32') { | ||
| const outputElementCount = sizeOfShape(outputShape); | ||
| const resultArray = []; |
There was a problem hiding this comment.
nit: resultArray -> data which aligns with "data" in your description.
src/constant.js
Outdated
| for (let i = 0; i < outputElementCount; i++) { | ||
| resultArray.push(start + i * step); | ||
| } | ||
| const resultToTensor = new Tensor(outputShape, resultArray); |
There was a problem hiding this comment.
nit: resultToTensor -> tensor
src/constant.js
Outdated
| } | ||
| const resultToTensor = new Tensor(outputShape, resultArray); | ||
| const output = cast(resultToTensor, type); | ||
| return output; |
There was a problem hiding this comment.
nit: merge the above two lines into one return cast(tensor, type);
|
|
||
| describe('test constant', function() { | ||
| function testConstant(start, step, outputShape, type, expected) { | ||
| const outputTensor = constant(start, step, outputShape, type); |
There was a problem hiding this comment.
There may be some fallout from the discussion of webmachinelearning/webnn#492, but it would be a minor incremental change. e.g.
const outputTensor = constant({dimensions: outputShape, dataType: dataType}, start, step);
(so, I don't see a reason to block either way?)
|
@BruceDai , please take another look. |
|
LGTM, thanks @mei1127 . |
@BruceDai @fdwr @huningxin @miaobin PTAL, thanks!