Adding dropout implmentation to RNNs

[ericdnielsen]

Not ready for review yet. Still need testing, etc. But getting into a PR for easier comparison.

---

This change is 

[ericdnielsen]

Tests added, ready for review

[caisq]

Review status: 0 of 1 LGTMs obtained

---

src/layers/recurrent.ts, line 1018 at r2 (raw file):

        this.dropoutMask = generateDropoutMask(() => {
                             return tfc.onesLike(inputs as Tensor);
                           }, this.dropout, training) as Tensor;

This can be simplified as () => tfc.onesLike(input as Tensor). Same elsewhere.

---

src/layers/recurrent.ts, line 1024 at r2 (raw file):

            generateDropoutMask(() => {
              return tfc.onesLike(prevOutput);
            }, this.recurrentDropout, training) as Tensor;

This can be simplified as () => tfc.onesLike(prevOutput). Same elsewhere.

---

src/layers/recurrent.ts, line 1474 at r2 (raw file):

Tensor[]

Optional: Would be slightly safer to type this as [Tensor, Tensor, Tensor]. Same below.

---

src/layers/recurrent.ts, line 2513 at r2 (raw file):

training = training)

s/training = training/training/

Same below.

---

src/layers/recurrent_test.ts, line 540 at r2 (raw file):

dropouts = [0.0, 0.1];

Can we add tests for recurrentDropout > 0 and < 1?

---

src/layers/recurrent_test.ts, line 548 at r2 (raw file):

1

For generality, can we make this a larger number?

---

src/layers/recurrent_test.ts, line 565 at r2 (raw file):

1

This should be 1 regardless of timeSteps, right? (No change required. Just confirming.)

---

src/layers/recurrent_test.ts, line 582 at r2 (raw file):

1

As above, we can make this a larger number?

---

src/layers/recurrent_test.ts, line 602 at r2 (raw file):

}

Add an else branch with assertions in it.

---

src/layers/recurrent_test.ts, line 912 at r2 (raw file):

}

this needs an else branch with assertions in it.

---

src/layers/recurrent_test.ts, line 1235 at r2 (raw file):

}

This needs an else branch with assertions in it.

---

Comments from Reviewable

[ericdnielsen]

Review status: 0 of 1 LGTMs obtained

---

src/layers/recurrent.ts, line 1018 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

        this.dropoutMask = generateDropoutMask(() => {
                             return tfc.onesLike(inputs as Tensor);
                           }, this.dropout, training) as Tensor;

This can be simplified as () => tfc.onesLike(input as Tensor). Same elsewhere.

Done.

---

src/layers/recurrent.ts, line 1024 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

            generateDropoutMask(() => {
              return tfc.onesLike(prevOutput);
            }, this.recurrentDropout, training) as Tensor;

This can be simplified as () => tfc.onesLike(prevOutput). Same elsewhere.

Done.

---

src/layers/recurrent.ts, line 1474 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

Tensor[]

Optional: Would be slightly safer to type this as [Tensor, Tensor, Tensor]. Same below.

Done.

---

src/layers/recurrent.ts, line 2513 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

training = training)

s/training = training/training/

Same below.

Done.

---

src/layers/recurrent_test.ts, line 540 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

dropouts = [0.0, 0.1];

Can we add tests for recurrentDropout > 0 and < 1?

discussed, offline.

---

src/layers/recurrent_test.ts, line 548 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

1

For generality, can we make this a larger number?

Done.

---

src/layers/recurrent_test.ts, line 565 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

1

This should be 1 regardless of timeSteps, right? (No change required. Just confirming.)

Done.

---

src/layers/recurrent_test.ts, line 582 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

1

As above, we can make this a larger number?

Done.

---

src/layers/recurrent_test.ts, line 602 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

}

Add an else branch with assertions in it.

Done.

---

src/layers/recurrent_test.ts, line 912 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

}

this needs an else branch with assertions in it.

Done.

---

src/layers/recurrent_test.ts, line 1235 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

}

This needs an else branch with assertions in it.

Done.

---

Comments from Reviewable

[caisq]

Review status: 0 of 1 LGTMs obtained

---

src/layers/recurrent.ts, line 1474 at r2 (raw file):

Previously, ericdnielsen wrote…

Done.

You say it's done. But I don't see any changes made here?

---

src/layers/recurrent_test.ts, line 542 at r3 (raw file):

for (const training of trainings) {

For tests like this, can we add the following check for the absence of memory leak?

After the 1st iteration, call tfc.memory().numTensors to get the number of tensors in the backend.
After the 2nd iteration, get that number again and assert that it should be equal to the previous number.

---

src/layers/recurrent_test.ts, line 566 at r3 (raw file):

1  *

Is there any reason to keep this 1 * here? Same below?

---

Comments from Reviewable

[ericdnielsen]

Review status: 0 of 1 LGTMs obtained

---

src/layers/recurrent.ts, line 1474 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

You say it's done. But I don't see any changes made here?

Ahh I changed the LSTM ones, not the GRU ones. Both are changed now.

---

src/layers/recurrent_test.ts, line 542 at r3 (raw file):

Previously, caisq (Shanqing Cai) wrote…

for (const training of trainings) {

For tests like this, can we add the following check for the absence of memory leak?

After the 1st iteration, call tfc.memory().numTensors to get the number of tensors in the backend.
After the 2nd iteration, get that number again and assert that it should be equal to the previous number.

Done.

---

src/layers/recurrent_test.ts, line 566 at r3 (raw file):

Previously, caisq (Shanqing Cai) wrote…

1  *

Is there any reason to keep this 1 * here? Same below?

I think, compared to the GRU & LSTM ones, where its not 1 *, the parallelism is useful.

---

Comments from Reviewable

[caisq]

Thanks!

---

Review status: complete! 1 of 1 LGTMs obtained

---

src/layers/recurrent.ts, line 2009 at r4 (raw file):

[Tensor, Tensor, Tensor];

Isn't this supposed to be four (instead of three) Tensors? Same in the line below.

---

Comments from Reviewable

[ericdnielsen]

Review status: complete! 1 of 1 LGTMs obtained

---

src/layers/recurrent.ts, line 2009 at r4 (raw file):

Previously, caisq (Shanqing Cai) wrote…

[Tensor, Tensor, Tensor];

Isn't this supposed to be four (instead of three) Tensors? Same in the line below.

Looks like it should be, not sure why TS & lint don't complain.

---

Comments from Reviewable