Fix bufferSync return type

[ahmedsabie]

The existing bufferSync always returned a float template type which throws away the type, so users of the function always assume they are working with a number array which is incorrect for string types.

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

---

This change is 

[pyu10055]

Reviewed 13 of 13 files at r1, all commit messages.
Reviewable status: complete! 1 of 1 approvals obtained (waiting on @lina128)

[lina128]

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

---

tfjs-backend-cpu/src/backend_cpu.ts line 150 at r1 (raw file):

  }

  bufferSync<R extends Rank, D extends DataType>(t: TensorInfo):

Can you add 'float32' as the default DataType here?

Code quote:

D 

[ahmedsabie]

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

---

tfjs-backend-cpu/src/backend_cpu.ts line 150 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Can you add 'float32' as the default DataType here?

This would reintroduce the problem of accidental cast to float since no compiler error would be generated, versus without default where you have to assert the default so it's an explicit decision. This is the same problem I ran into trying to add string support to SparseToDense

[lina128]

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

---

tfjs-backend-cpu/src/backend_cpu.ts line 150 at r1 (raw file):

Previously, ahmedsabie wrote…

This would reintroduce the problem of accidental cast to float since no compiler error would be generated, versus without default where you have to assert the default so it's an explicit decision. This is the same problem I ran into trying to add string support to SparseToDense

Can you specify what error you saw when adding string support to SparseToDense? Or maybe send me that PR with the CI log? Thanks

[ahmedsabie]

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

---

tfjs-backend-cpu/src/backend_cpu.ts line 150 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Can you specify what error you saw when adding string support to SparseToDense? Or maybe send me that PR with the CI log? Thanks

The error involved defaultValue being type asserted into a number which compiled due to bufferSync always returning a float32 dtype implicitly. That's when I found this bug with bufferSync storing string values as a float32 dtype so that's why we need dtype template added.

I suggest we don't put a default because it forces the user to choose what type they are working with and it is explicit to the next user. The same problem above would occur if the default was float32 and the previous coder didn't explicitly specify float

[lina128]

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

---

tfjs-backend-cpu/src/backend_cpu.ts line 158 at r1 (raw file):

        const strings = (data as Uint8Array[]).map(d => util.decodeString(d));
        return buffer(t.shape as ShapeMap[R], t.dtype, strings) as
            TensorBuffer<R, D>;

Will it throw error if we force cast type as string here and just use default everywhere else? Like this:

        return buffer(t.shape as ShapeMap[R], t.dtype, strings) as
            TensorBuffer<R, string>;

or

        return buffer(t.shape as ShapeMap[R], t.dtype, strings) as
            any as TensorBuffer<R, string>;

Code quote:

TensorBuffer<R, D>

[ahmedsabie]

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

---

tfjs-backend-cpu/src/backend_cpu.ts line 158 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Will it throw error if we force cast type as string here and just use default everywhere else? Like this:

        return buffer(t.shape as ShapeMap[R], t.dtype, strings) as
            TensorBuffer<R, string>;

or

        return buffer(t.shape as ShapeMap[R], t.dtype, strings) as
            any as TensorBuffer<R, string>;

It's a compile error because templates are inputs, the user could theoretically pass in a string input but ask for a float return type which is why you can't make an assertion to string

[lina128]

LGTM

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