Add Unique op

[jinjingforever]

This is for issue #1866 (per Ping's suggestion)

• To match the TF implementation, the op supports int32, bool, and string. In theory it also supports floats, but I guess it is not that useful in practice.
• The op API also accepts an "axis" parameter, but it is not used now. It is there to support UniqueV2 that we might want to support in the future.
• For now, the webgl backend will forwarded calculations to cpu backend.
• Hook it up with converter. Please check if there is anything that I missed.

---

This change is 

[lina128]

Hi Jing, a high level question, is uniqueV2 a more generic case of unique? If so, it is probably a better idea to implement uniqueV2, which will be able to handle unique. Otherwise, if you implement uniqueV2 later, the more specific implementation here may be thrown away, which is a waste of effort. WDYT?

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

[pyu10055]

Thank Jing! This is a great addition. I agree with Na, add support both unique and uniqueV2 would be great. You can take a look at the TF core implmentation https://source.corp.google.com/piper///depot/google3/third_party/tensorflow/core/kernels/unique_op.cc?q=uniquev2%20&sq=package:piper%20file:%2F%2Fdepot%2Fgoogle3%20-file:google3%2Fexperimental

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @pyu10055, and @tafsiri)

[annxingyuan]

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @jinjingforever, and @tafsiri)

---

tfjs-backend-webgl/src/kernels/Unique.ts, line 31 at r1 (raw file):

  // For now, always forward calculation to the CPU backend.
  const values = backend.readSync(x.dataId);

Can you throw a warning in the console that this kernel is calling readSync? readSync locks the UI.

[pyu10055]

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan and @tafsiri)

---

tfjs-backend-webgl/src/kernels/Unique.ts, line 31 at r1 (raw file):

Previously, annxingyuan (Ann Yuan) wrote…

Can you throw a warning in the console that this kernel is calling readSync? readSync locks the UI.

can this function be async?

[annxingyuan]

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

---

tfjs-backend-webgl/src/kernels/Unique.ts, line 31 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

can this function be async?

We don't support async kernels yet so I don't think we can call async read here. Although it would be great to profile unique in the context of a model on the WebGL backend.

[jinjingforever]

Thanks for comments and pointers! I updated the implementations to support UniqueV2.

PTAL

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan and @tafsiri)

---

tfjs-backend-webgl/src/kernels/Unique.ts, line 31 at r1 (raw file):

Previously, annxingyuan (Ann Yuan) wrote…

We don't support async kernels yet so I don't think we can call async read here. Although it would be great to profile unique in the context of a model on the WebGL backend.

Added the warning. I will ping you guys offline about profiling. Thanks!

[pyu10055]

thanks for adding axis support.

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @jinjingforever, and @tafsiri)

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 27 at r2 (raw file):

} {
  // Normalize and validate axis.
  const $axis = util.parseAxisParam(axis, shape)[0];

need to make sure axis is less than rank of the tensor, c++ also supports negative axis, which is counting from the end.

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 39 at r2 (raw file):

  // For example, for a 4D tensor with shape=[2, 3, 5, 4] and axis=2, the
  // newShape would be: [2*3, 5, 4].
  const newShape = [1, shape[0], 1];

do we need to make sure shape is at least 1D?

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 63 at r2 (raw file):

    // Extract values along the axis.
    let element: string;
    if (is1DTensor) {

do you need to special handle 1D case? should the m and n both are 1?

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 73 at r2 (raw file):

        }
      }
      element = axisValues.toString();

this depends on the implementation of array toString method, maybe better to implement your own to ensure future compatibility.

---

tfjs-backend-webgl/src/kernels/Unique.ts, line 35 at r2 (raw file):

  // For now, always forward calculation to the CPU backend.
  console.warn(
      'WARNING: ', 'UI might be locked temparaily as data is being downloaded');

typo: temporarily

---

tfjs-core/src/ops/unique_test.ts, line 65 at r2 (raw file):

  });

  it('2d tensor with axis=1', async () => {

can you add 3d tensor tests as well? thanks

[jinjingforever]

Thanks for the review Ping! PTAL

(Also added some code comments in unique_impl.ts to hopefully explain the algorithm better)

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @pyu10055, and @tafsiri)

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 27 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

need to make sure axis is less than rank of the tensor, c++ also supports negative axis, which is counting from the end.

Thanks. Actually the parseAxisParam function does the checks you mentioned already:) It also handles negative axis.

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 39 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

do we need to make sure shape is at least 1D?

Done. Added an assert in the op

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 63 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

do you need to special handle 1D case? should the m and n both are 1?

You are correct, but given that 1D input is probably the most common, it might help to improve its performance by directly setting the element value instead of going through creating an array and adding one element (below)

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 73 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

this depends on the implementation of array toString method, maybe better to implement your own to ensure future compatibility.

Good point. Done

---

tfjs-backend-webgl/src/kernels/Unique.ts, line 35 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

typo: temporarily

Done

---

tfjs-core/src/ops/unique_test.ts, line 65 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

can you add 3d tensor tests as well? thanks

Done.

[pyu10055]

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @jinjingforever, and @tafsiri)

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 27 at r2 (raw file):

Previously, jinjingforever (Jing Jin) wrote…

Thanks. Actually the parseAxisParam function does the checks you mentioned already:) It also handles negative axis.

nice, thanks!

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 73 at r2 (raw file):

Previously, jinjingforever (Jing Jin) wrote…

Good point. Done

For strings arrays ['a,b', 'c'] and ['a', 'b,c'], their value would be the same?

[jinjingforever]

Thank you Ping! PTAL.

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @pyu10055, and @tafsiri)

---

tfjs-backend-cpu/src/kernels/Unique_impl.ts, line 73 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

For strings arrays ['a,b', 'c'] and ['a', 'b,c'], their value would be the same?

We are actually ok here because the "values" stored in inputBuffer are raw byte arrays instead of actual strings. I added a test to make sure this works.

Your comment also reminded me that the code at the end of this method (convert string back to bytes) are not necessary because outputBuffer.values are bytes too:) Removed.

[pyu10055]

Reviewed 10 of 22 files at r1, 8 of 12 files at r2, 1 of 4 files at r3, 3 of 3 files at r4.
Reviewable status: complete! 1 of 1 approvals obtained (waiting on @annxingyuan, @pyu10055, and @tafsiri)