Implement initialState support in Bidirectional

[caisq]

Also in this PR:

• Fix serialization of models consisting of initial-state bidirectional layers

Fixes: tensorflow/tfjs#483

FEATURE

---

This change is 

[davidsoergel]

Reviewable status: 0 of 1 approvals obtained (waiting on @caisq, @nsthorat, @davidsoergel, @ericdnielsen, @dsmilkov, and @bileschi)

---

src/layers/wrappers.ts, line 442 at r1 (raw file):

      const fullInput = [inputs].concat(additionalInputs);
      const fullInputSpec = this.inputSpec.concat(additionalSpecs);
      // Perform the call temporarily and replace inputSpec.

This is clever but feels a little hacky. Would it make sense instead to add an optional additionalSpecs argument to Layer.apply()?

---

src/layers/wrappers_test.ts, line 411 at r1 (raw file):

Quoted 5 lines of code…

  let initState1: SymbolicTensor;
  let initState2: SymbolicTensor;
  let bidiLayer: Bidirectional;
  let x: SymbolicTensor;
  let y: SymbolicTensor[];

I think it's preferable to make the tests independent of each other-- i.e. createLayer can return these values (and you can readily assign them to local variables within each test using destructuring).

[caisq]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @caisq, @nsthorat, @ericdnielsen, @dsmilkov, and @bileschi)

---

src/layers/wrappers.ts, line 442 at r1 (raw file):

Previously, davidsoergel (David Soergel) wrote…

This is clever but feels a little hacky. Would it make sense instead to add an optional additionalSpecs argument to Layer.apply()?

This code is ported from keras / tf.keras. The reason why it's done this way is that when you have initial states, symbolic and non-symbolic calls to apply() (python's call) behave in slightly different ways. The symbolic call includes the SymbolicTensors for initial states in the first arg, whereas the non-symbolic call includes the Tensors for initial states in the second argument (kwargs). Hence this need for handling InputSpec differently. Removing this hacky code requires some structural changes to the Layer base class, which I think is risky and outside the scope of this PR. I added a comment and a TODO item here.

---

src/layers/wrappers_test.ts, line 411 at r1 (raw file):

Previously, davidsoergel (David Soergel) wrote…

  let initState1: SymbolicTensor;
  let initState2: SymbolicTensor;
  let bidiLayer: Bidirectional;
  let x: SymbolicTensor;
  let y: SymbolicTensor[];

I think it's preferable to make the tests independent of each other-- i.e. createLayer can return these values (and you can readily assign them to local variables within each test using destructuring).

I agree. Done. Thanks for the suggestion.