ensure clone is done only for not kept tensors

[pyu10055]

fix issue #3739
#3741
To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.

---

This change is 

[lina128]

Reviewable status: 0 of 1 approvals obtained (waiting on @lina128 and @pyu10055)

---

tfjs-converter/src/operations/executors/control_executor.ts, line 110 at r1 (raw file):

    case 'LoopCond': {
      let pred = getParamValue('pred', node, tensorMap, context) as tfc.Tensor;
      if (!pred.kept) {

Can we have some annotation of this logic?

---

tfjs-converter/src/operations/executors/control_executor.ts, line 111 at r1 (raw file):

      let pred = getParamValue('pred', node, tensorMap, context) as tfc.Tensor;
      if (!pred.kept) {
        pred = pred.clone();

Let's not use chained op, change to pred = clone(pred), this will allow proper tree shaking.

---

tfjs-converter/src/operations/executors/control_executor.ts, line 111 at r1 (raw file):

      let pred = getParamValue('pred', node, tensorMap, context) as tfc.Tensor;
      if (!pred.kept) {
        pred = pred.clone();

Also, better to make pred immutable and dispose it in the !kept condition because clone will be returned, not the original tensor. Or does it get cleaned up somewhere?

[pyu10055]

Reviewable status: 0 of 1 approvals obtained (waiting on @lina128)

---

tfjs-converter/src/operations/executors/control_executor.ts, line 110 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Can we have some annotation of this logic?

Done.

---

tfjs-converter/src/operations/executors/control_executor.ts, line 111 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Let's not use chained op, change to pred = clone(pred), this will allow proper tree shaking.

Done.

---

tfjs-converter/src/operations/executors/control_executor.ts, line 111 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Also, better to make pred immutable and dispose it in the !kept condition because clone will be returned, not the original tensor. Or does it get cleaned up somewhere?

this is disposed in the executor.

[lina128]

Reviewable status: 0 of 1 approvals obtained (waiting on @lina128 and @pyu10055)

---

tfjs-converter/src/operations/executors/control_executor.ts, line 110 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

Done.

What happens if the tensor is marked as keep, and we still clone? Can we add this reason to the annotation too? Also can we add a test, something that will throw the error in the bug report and can show that after this fix, it solves it?

[lina128]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @lina128 and @pyu10055)

---

tfjs-converter/src/operations/executors/control_executor.ts, line 110 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

What happens if the tensor is marked as keep, and we still clone? Can we add this reason to the annotation too? Also can we add a test, something that will throw the error in the bug report and can show that after this fix, it solves it?

Hi Ping, once the annotation is revised to explain possible fail case and how using keep check prevents it. LGTM.

---

tfjs-converter/src/operations/executors/control_executor.ts, line 111 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

Done.

Hi Ping, once the