completed lstm_cell

[mei1127]

@huningxin @fdwr @shiyi9801 @BruceDai PTAL, thanks!

[BruceDai]

Please remove this blank line.

[mei1127]

OK

[BruceDai]

Suggested change

	const starts = layout === 'iofg' ? {i: 0, o: hiddenSize, f: 2* hiddenSize, g: 3*hiddenSize} :
	const starts = layout === 'iofg' ? {i: 0, o: hiddenSize, f: 2 * hiddenSize, g: 3 * hiddenSize} :
	{i: 0, f: hiddenSize, g: 2 * hiddenSize, o: 3 * hiddenSize};

[BruceDai]

Please use it() method.

[BruceDai]

Prefer keep 2 space indentations.

[fdwr]

Prefer keep 2 space indentations.

👍 Yeah, although I much prefer 4-space indents over 2-space indents to more clearly/accurately/quickly see the overall hierarchy of blocks, even more important is consistency within a codebase, and since the rest of the files are 2-space, then I'd do so here too (a mix of both 4-space and 2-space makes it harder to follow the levels too :'-( ).

There are lots of possible ways to do it, but picking one and sticking with it is better than switching between 3 different kinds of indentation. e.g.

Linear flow (uses more vertical space, but it's highly scannable for the eye to quickly locate code, efficient to glean overall block structure, consistently symmetric in constructs, and easily diffable)

    const hiddenState = new Tensor(                          // ->
      [batchSize, hiddenSize],                               //  0
      new Float32Array(batchSize * hiddenSize).fill(0),      //  1
    );                                                       // <-
    const cellState = new Tensor(                            // ->
      [batchSize, hiddenSize],                               //  0
      new Float32Array(batchSize * hiddenSize).fill(0),      //  1
    );                                                       // <-
    const activations = [                                    // ->
      relu,                                                  //  0
      relu,                                                  //  1
      relu,                                                  //  2
    ];                                                       // <-
    const outputs = lstmCell(
      input,
      weight,
      recurrentWeight,
      hiddenState,
      cellState,
      hiddenSize,
      {bias, recurrentBias, peepholeWeight, activations}
    );

Compact zig-zag ragged wrap with variable indent for nested lines (slower to scan and spot identifiers, and terrible for diffing, but shaves vertical space):

    const hiddenState = new Tensor([batchSize, hiddenSize],
                                   new Float32Array(batchSize * hiddenSize).fill(0));
    const cellState = new Tensor([batchSize, hiddenSize],
                                 new Float32Array(batchSize * hiddenSize).fill(0));
    const activations = [relu, relu, relu];
    const outputs = lstmCell(input, weight, recurrentWeight, hiddenState, cellState, hiddenSize,
                             {bias, recurrentBias, peepholeWeight, activations});

Compact zig-zag ragged wrap with uniform indentation for nested lines (slower to scan, bad for diffing albeit it a little less than #‌1, and shaves less vertical space):

    const hiddenState = new Tensor(
      [batchSize, hiddenSize],
      new Float32Array(batchSize * hiddenSize).fill(0));
    const cellState = new Tensor(
      [batchSize, hiddenSize],
      new Float32Array(batchSize * hiddenSize).fill(0));
    const activations = [
      relu, relu, relu];
    const outputs = lstmCell(
      input, weight, recurrentWeight, hiddenState, cellState, hiddenSize,
      {bias, recurrentBias, peepholeWeight, activations});

[BruceDai]

Thanks @mei1127.
LGTM with some nits, please do some modifications.

[fdwr]

Minor comments. Thanks Mei.

[fdwr]

Would be easier for future readers to read if indentation was aligned consistently:

Suggested change

	const starts = layout === 'iofg' ? {i: 0, o: hiddenSize, f: 2 * hiddenSize, g: 3 *hiddenSize} :
	{i: 0, f: hiddenSize, g: 2 * hiddenSize, o: 3 * hiddenSize};
	const starts = layout === 'iofg' ? {i: 0, o: hiddenSize, f: 2 * hiddenSize, g: 3 *hiddenSize} :
	{i: 0, f: hiddenSize, g: 2 * hiddenSize, o: 3 * hiddenSize};

[fdwr]

Prefer keep 2 space indentations.

👍 Yeah, although I much prefer 4-space indents over 2-space indents to more clearly/accurately/quickly see the overall hierarchy of blocks, even more important is consistency within a codebase, and since the rest of the files are 2-space, then I'd do so here too (a mix of both 4-space and 2-space makes it harder to follow the levels too :'-( ).

There are lots of possible ways to do it, but picking one and sticking with it is better than switching between 3 different kinds of indentation. e.g.

Linear flow (uses more vertical space, but it's highly scannable for the eye to quickly locate code, efficient to glean overall block structure, consistently symmetric in constructs, and easily diffable)

    const hiddenState = new Tensor(                          // ->
      [batchSize, hiddenSize],                               //  0
      new Float32Array(batchSize * hiddenSize).fill(0),      //  1
    );                                                       // <-
    const cellState = new Tensor(                            // ->
      [batchSize, hiddenSize],                               //  0
      new Float32Array(batchSize * hiddenSize).fill(0),      //  1
    );                                                       // <-
    const activations = [                                    // ->
      relu,                                                  //  0
      relu,                                                  //  1
      relu,                                                  //  2
    ];                                                       // <-
    const outputs = lstmCell(
      input,
      weight,
      recurrentWeight,
      hiddenState,
      cellState,
      hiddenSize,
      {bias, recurrentBias, peepholeWeight, activations}
    );

Compact zig-zag ragged wrap with variable indent for nested lines (slower to scan and spot identifiers, and terrible for diffing, but shaves vertical space):

    const hiddenState = new Tensor([batchSize, hiddenSize],
                                   new Float32Array(batchSize * hiddenSize).fill(0));
    const cellState = new Tensor([batchSize, hiddenSize],
                                 new Float32Array(batchSize * hiddenSize).fill(0));
    const activations = [relu, relu, relu];
    const outputs = lstmCell(input, weight, recurrentWeight, hiddenState, cellState, hiddenSize,
                             {bias, recurrentBias, peepholeWeight, activations});

Compact zig-zag ragged wrap with uniform indentation for nested lines (slower to scan, bad for diffing albeit it a little less than #‌1, and shaves less vertical space):

    const hiddenState = new Tensor(
      [batchSize, hiddenSize],
      new Float32Array(batchSize * hiddenSize).fill(0));
    const cellState = new Tensor(
      [batchSize, hiddenSize],
      new Float32Array(batchSize * hiddenSize).fill(0));
    const activations = [
      relu, relu, relu];
    const outputs = lstmCell(
      input, weight, recurrentWeight, hiddenState, cellState, hiddenSize,
      {bias, recurrentBias, peepholeWeight, activations});

[fdwr]

Full names are nice, like return [hiddenState, cellState]. Plus you already spell these names hiddenState and cellStates elsewhere below, which would be more consistent.

[Mei0211]

Maybe hiddenStateNew could be better for understand and differentiate from the previous state？

[fdwr]

The error message is split.