Clone() doesn't actually clone?

[TheNewSound]

To get help from the community, check out our Google group.

TensorFlow.js version

Latest (0.14.1)

Browser version

Latest Firefox + Chrome

Describe the problem or feature request

Clone() doesn't actually clone.

Code to reproduce the bug / link to feature request

You can run the following code on the tfjs API console (https://js.tensorflow.org/api/0.14.1/#clone)

const x = tf.tensor([3, 2, 1]);

x.clone().data().then(response => {
     const sorted = response.sort();
     console.log(sorted); // gets called second
     console.log(x.dataSync()); // gets called last
});
console.log(x.dataSync()); // gets called first

Or seen synchronous:

const x = tf.tensor([3, 2, 1]);
console.log(x.dataSync()); //gets called first
const data = tf.clone(x).dataSync();
const sorted = data.sort();
console.log(sorted); // gets called second
console.log(x.dataSync()); // gets called last

Output:

3,2,1
1,2,3
1,2,3

Expected output:

3,2,1
1,2,3
3,2,1

[davidsoergel]

I think the problem is this: Tensors are supposed to be immutable, so clone() returns a reference to the existing data without actually copying it-- which should be safe. However, when using the CPU backend, readSync() just returns the underlying array, which actually is mutable. In the original example above, data.sort() is sorting the Float32Array in place.

This demonstrates the mutability more directly:

const x = tf.tensor([3, 2, 1]);
const data = x.dataSync();
console.log(x.dataSync());
console.log(data instanceof Float32Array);
data[1] = 54;
console.log(x.dataSync());

Output:

3,2,1
true
3,54,1

So, to produce the expected behavior, I think the CPU readSync() method needs to return a copy of the data at backend_cpu.ts:169. (The GPU version returns a copy anyway).
@dsmilkov, WDYT?

[dsmilkov]

Clone makes a shallow copy of the tensor for performance reasons.

If we do a true copy upon dataSync(), it will come at a cost of memory read/writes and increased garbage collection. We'll have to do it for the WebGL backend as well, for tensors that haven't been uploaded to GPU yet, or have recently been downloaded.

I think we should start with documenting clone() better (emphasize that it does shallow copy) and revisit this if it becomes a more frequent issue.

[TheNewSound]

So in what other way can one achieve to sort the array / tensor without modifying the original tensor?

Use a variable tensor and clone() it? As variable tensors are regarded as mutable, the expected output of clone() on a variable tensor should be a deep clone?

[dsmilkov]

Slice the TypedArray using TypedArray.slice().

In this case:

const myData = x.dataSync().slice();
myData.sort()