[wasm] Add Reverse kernel.

[annxingyuan]

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

---

This change is 

[tafsiri]

LGTM with a small comments.

Reviewed 6 of 6 files at r1.
Reviewable status: complete! 1 of 1 approvals obtained (waiting on @annxingyuan, @pyu10055, and @tafsiri)

---

tfjs-backend-wasm/src/cc/kernels/Reverse.cc, line 55 at r1 (raw file):

        tfjs::util::offset_to_loc(i, out_strides);

    std::vector<size_t> in_loc = out_loc;

just curious. Why not just directly assign in_loc to the value on the line above, out_loc doesn't seem be used for anything else.

---

tfjs-backend-wasm/src/kernels/Reverse.ts, line 44 at r1 (raw file):

}): TensorInfo {
  const {inputs, backend, attrs} = args;
  const {x} = inputs as {} as ReverseInputs;

optional: a tip i've found recently, if you cast the kernelFunc on line 69 to a ForwardFunc, then you can make this function directly take ReverseInputs and ReverseAttrs and not have to cast here.

---

tfjs-backend-wasm/src/kernels/Reverse.ts, line 66 at r1 (raw file):

}

registerKernel({

I think we wanted to move to doing registration outside of the files defining the kernel for consistency with other backends (and for potential tree-shakeability of wasm in future as well). Might as well not add new ones in the older style.

[annxingyuan]

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

---

tfjs-backend-wasm/src/cc/kernels/Reverse.cc, line 55 at r1 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

just curious. Why not just directly assign in_loc to the value on the line above, out_loc doesn't seem be used for anything else.

Done

---

tfjs-backend-wasm/src/kernels/Reverse.ts, line 44 at r1 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

optional: a tip i've found recently, if you cast the kernelFunc on line 69 to a ForwardFunc, then you can make this function directly take ReverseInputs and ReverseAttrs and not have to cast here.

Done

---

tfjs-backend-wasm/src/kernels/Reverse.ts, line 66 at r1 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

I think we wanted to move to doing registration outside of the files defining the kernel for consistency with other backends (and for potential tree-shakeability of wasm in future as well). Might as well not add new ones in the older style.

Done

[annxingyuan]

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

---

tfjs-backend-wasm/src/kernels/Reverse.ts, line 44 at r1 (raw file):

Previously, annxingyuan (Ann Yuan) wrote…

Done

Actually I'm not sure I understand - it doesn't seem like the types are compatible? What am I missing?

[annxingyuan]

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

---

tfjs-backend-wasm/src/kernels/Reverse.ts, line 44 at r1 (raw file):

Previously, annxingyuan (Ann Yuan) wrote…

Actually I'm not sure I understand - it doesn't seem like the types are compatible? What am I missing?

OH i see - casting as KernelFunc works!

[annxingyuan]

@tafsiri Hey Yannick - would you mind taking another look at this one? I've changed the WASM implementation of clone to reflect our last conversation.

[lina128]

LGTM

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

[tafsiri]

Reviewed 8 of 9 files at r2.
Reviewable status: complete! 2 of 1 approvals obtained (waiting on @annxingyuan, @pyu10055, and @tafsiri)

---

tfjs-backend-wasm/src/register_all_kernels.ts, line 22 at r2 (raw file):

import {KernelConfig, registerKernel} from '@tensorflow/tfjs-core';

import {reverseConfig} from './kernels/Reverse';

After this PR could you do a followup one converting all the existing kernels to use this style?

[annxingyuan]

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

---

tfjs-backend-wasm/src/register_all_kernels.ts, line 22 at r2 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

After this PR could you do a followup one converting all the existing kernels to use this style?

Yes sounds good.