modularize reverse ops

[tafsiri]

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.

---

This change is 

[lina128]

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

---

tfjs-core/src/gradients/Reverse_grad.ts, line 28 at r1 (raw file):

    const axes = parseAxisParam(dims, dy.shape);

    return {x: () => dy.reverse(axes)};

Remove chained op.

---

tfjs-core/src/ops/reverse.ts, line 66 at r1 (raw file):

    const axes = parseAxisParam(axis, $x.shape);
    if ($x.rank === 0) {
      return $x.clone();

Remove chained op.

---

tfjs-core/src/public/chained_ops/reverse.ts, line 23 at r1 (raw file):

declare module '../../tensor' {
  interface Tensor<R extends Rank = Rank> {
    reverse<T extends Tensor>(this: T, axis?: number|number[]): T;

Curious, does having this: T help avoiding type cast below?

[tafsiri]

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

---

tfjs-core/src/gradients/Reverse_grad.ts, line 28 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done

---

tfjs-core/src/ops/reverse.ts, line 66 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done

---

tfjs-core/src/public/chained_ops/reverse.ts, line 23 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Curious, does having this: T help avoiding type cast below?

I'm not sure (and may not fully understand the question), do you mean using T instead of Tensor here and below? Ultimately the types used by the implementation below should match the types in the signature here.

[lina128]

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

---

tfjs-core/src/public/chained_ops/reverse.ts, line 23 at r1 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

I'm not sure (and may not fully understand the question), do you mean using T instead of Tensor here and below? Ultimately the types used by the implementation below should match the types in the signature here.

I mean why not just reverse<T extends Tensor>(axis?: number|number[]): T, same for below.

[tafsiri]

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

---

tfjs-core/src/public/chained_ops/reverse.ts, line 23 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

I mean why not just reverse<T extends Tensor>(axis?: number|number[]): T, same for below.

ah yes, this helps avoid needing to cast the return value from reverse with as T in the implementation below. Without it typescript will infer this to be of type Tensor<any> (not Tensor). At the call site it will be the same as what you typed above. The ts language feature is described here https://www.typescriptlang.org/docs/handbook/functions.html#this-parameters

[lina128]

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

---

tfjs-core/src/public/chained_ops/reverse.ts, line 23 at r1 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

ah yes, this helps avoid needing to cast the return value from reverse with as T in the implementation below. Without it typescript will infer this to be of type Tensor<any> (not Tensor). At the call site it will be the same as what you typed above. The ts language feature is described here https://www.typescriptlang.org/docs/handbook/functions.html#this-parameters

Got it. Thanks for the detailed explanation, Yannick! I remember you mentioned this once before, just want to confirm. Thanks!