Avoid disposing intermediate tensor in grad

[Lewuathe]

Purpose

Fix tensorflow/tfjs#227.
As well as other ops, grad should not dispose intermediate result. We should be able to call dispose explicitly or use tidy in such case.

---

This change is 

[bileschi]

Thanks for looking into this!

[bileschi]

Please use the same syntax for checking both data[0] and data[1]

[nsthorat]

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

src/gradients.ts, line 146 at r1 (raw file):

                'shape returned by f([x1,...])');
      }
      value.dispose();

ah this is going to introduce a memory leak, but your obversation is absolutely correct. Can you do this with a tf.tidy() instead of the dispose?

---

src/tensor_test.ts, line 1103 at r1 (raw file):

Previously, bileschi (Stanley Bileschi) wrote…

Please use the same syntax for checking both data[0] and data[1]

You should use expectArraysClose. This method can assert a tensor and a vanilla JS array have the same values.

---

Comments from Reviewable

[nsthorat]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

src/gradients.ts, line 146 at r1 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

ah this is going to introduce a memory leak, but your obversation is absolutely correct. Can you do this with a tf.tidy() instead of the dispose?

To be clearer:

return tidy(() => {

  const {value, grads} = ENV.engine.gradients(() => f(...args), args, dy);
  // ...
  return grads;
});

---

Comments from Reviewable

[dsmilkov]

Thanks! Rerunning travis since we had a strange test failure last time

---

Reviewed 1 of 2 files at r1.
Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[dsmilkov]

Reviewed 1 of 2 files at r2.
Review status: 1 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

src/tensor_test.ts, line 1093 at r2 (raw file):

  });

  it('grad with second derivative', () => {

can you move this test outside of the describe('tensor.toString') and inside the describeWithFlags('tensor', ALL_ENVS, () => { in the same file above since it does do mathematical operations. This is why the travis fails: https://api.travis-ci.org/v3/job/377450573/log.txt

---

Comments from Reviewable

[Lewuathe]

Review status: 1 of 2 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

src/tensor_test.ts, line 1093 at r2 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

can you move this test outside of the describe('tensor.toString') and inside the describeWithFlags('tensor', ALL_ENVS, () => { in the same file above since it does do mathematical operations. This is why the travis fails: https://api.travis-ci.org/v3/job/377450573/log.txt

Oh, good catch. I'm wondering why the test keeps failing. Thanks I'm going to fix it.

---

Comments from Reviewable