[WebGL backend] Add max 1D texture dimension flag #6808

Linchenn · 2022-08-31T01:18:55Z

Bug summary

Problem

Mali GPU could not provide interpolation values with enough precision for long skinny triangles, proved by the demo. The varying from the interpolation values will be used to calculate coords (in type) for sampling, finally causing sampling mismatch.

Standalone code to reproduce the issue

tf.ENV.set('WEBGL_PACK_DEPTHWISECONV', true)

let w = Array.from({length: 3 * 3 * 816}, () => Math.random())
let x = Array.from({length: 12 * 10 * 816}, () => Math.random())

let inputs = {
    filter: tf.tensor(w, [3, 3, 816, 1]),
    x: tf.tensor(x, [1, 12, 10, 816]),
    strides: 1,
    pad: [[0, 0], [1, 1], [1, 1], [0, 0]],
    dataFormat: "channelsLast",
    dilations: 1,
    activation: 'relu'
};

tf.setBackend('webgl')
tf.ENV.set('WEBGL_MAX_TEXTURE_SIZE', 4096)
let out_4096 = tf.fused.depthwiseConv2d(inputs);

tf.ENV.set('WEBGL_MAX_TEXTURE_SIZE', 2048)
inputs.x = tf.tensor(x, [1, 12, 10, 816])
inputs.filter = tf.tensor(w, [3, 3, 816, 1])
let out_2048 = tf.fused.depthwiseConv2d(inputs);

tf.setBackend('cpu')
inputs.x = tf.tensor(x, [1, 12, 10, 816])
inputs.filter = tf.tensor(w, [3, 3, 816, 1])
let out_reference = tf.fused.depthwiseConv2d(inputs);

const doTensorsDiffer = function(t0, t1) {
	return tf.any(tf.greater(tf.abs(tf.sub(t0, t1)), tf.scalar(1e-2))).dataSync()[0];
}

console.log("Default and 2048 differ? " + doTensorsDiffer(out_4096, out_2048));
console.log("Reference and 2048 differ? " + doTensorsDiffer(out_reference, out_2048));
console.log("Reference and 4096 differ? " + doTensorsDiffer(out_reference, out_4096));

The three differs are expected to be 0, while Mali GPU generates 1 for the first and the third.

Findings

Take the texture with physical size of [3672, 1] for example. As the second dimension increase, the precision becomes better. Actually, when the second dimension is increased to 2, the precision for calculating coords is enough. (reference data: for each table: the second column is the computed index from varying values; the third and forth columns are ground-truth; the fifth column is the difference between indices. You could find all difference between indices are 0.)

Solution

This PR expose a flag, WEBGL_MAX_TEXTURE_DIMENSION_FOR_1D_TEXTURE. For 1D texture (physical height or physical width is 1), if the physical length of any texture edges exceed the threshold, the other edge will be increased to 2.

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

This change is

Linchenn · 2022-08-31T01:21:08Z

cc @shanumantesc

qjia7

LGTM, thanks.

Can you also add the depthwiseConv2d case described in the bug for webgl?

Linchenn

Done. Thanks!

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

qjia7

Forget to submit the test file?

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

pyu10055

Thanks Lin for investigating and fixing this deep hidden bug. I am curious why we take the approach to double the dimension of shape 1, instead of using the squarish function?
I assume the squarish function to produce smaller texture size and potentially reduce the computation if the texture is output.

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

tfjs-backend-webgl/src/webgl_util.ts line 446 at r1 (raw file):

      env().getNumber('WEBGL_MAX_TEXTURE_DIMENSION_FOR_1D_TEXTURE') *
      (isPacked ? 2 : 1);
  if (textureShape[0] > maxDimFor1DTex || textureShape[1] > maxDimFor1DTex) {

It would be good to separate these two conditions to avoid unnecessary computation later.

tfjs-backend-webgl/src/webgl_util.ts line 449 at r1 (raw file):

    // For 1D texture, if the length exceeds maxDimFor1DTex, the texture will be
    // upgraded to 2D with a physical shape as [length, 2] or [2, length].
    textureShape[0] = Math.max(isPacked ? 4 : 2, textureShape[0]);

should the packed texture be 2?

… getVenderInfo

Linchenn

Thank you Ping for the suggestion! Updated the algorithm to squarify the shape.

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

tfjs-backend-webgl/src/webgl_util.ts line 446 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

It would be good to separate these two conditions to avoid unnecessary computation later.

Updated.

tfjs-backend-webgl/src/webgl_util.ts line 449 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

should the packed texture be 2?

Using squarifying now.

Linchenn

@qjia7 Since this PR only updates the getTextureShapeFromLogicalShape function, I just added some tests for it.

For the depthwise's mismatch bug here, we do not guarantee this PR could fix it. Instead, this PR expose the flags to users and let users to tune the flag and correct the mismatch.

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

pyu10055

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

tfjs-backend-webgl/src/webgl_util.ts line 374 at r2 (raw file):

      env().getNumber('WEBGL_MAX_SIZE_FOR_NARROW_TEXTURE');
  if (maxSizeForNarrorTex === Infinity &&
      env().getBool('WEBGL_AUTO_SQUARIFY_NARROW_TEXTURE_SHAPE_FOR_MALI_GPU')) {

we need to detect the GPU type if it is MALI, to ensure it only applies to MALI GPU.
or you can change the name to WEBGL_AUTO_SQUARIFY_NARROW_TEXTURE_SHAPE
so it is user's responsibility to detect MALI GPU.

tfjs-backend-webgl/src/webgl_util.ts line 404 at r2 (raw file):

      (textureShape: [number, number]) => {
        return Math.max(...textureShape) > maxSizeForNarrorTex &&
            Math.min(...textureShape) <= (isPacked ? 2 : 1);

if the texture is a 0 shaped tensor, we should not change it.
Which means Math.min(...textureShape) === (isPacked ? 2 : 1)

tfjs-backend-webgl/src/webgl_util.ts line 420 at r2 (raw file):

      logShape.length === 2 && logShape[0] <= maxTexSize &&
      logShape[1] <= maxTexSize &&
      !isLongNarrowTex(logShape as [number, number])) {

cache this boolean value, since it is checked in multiple places.

Linchenn

Thank you Ping!

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

tfjs-backend-webgl/src/webgl_util.ts line 374 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

we need to detect the GPU type if it is MALI, to ensure it only applies to MALI GPU.
or you can change the name to WEBGL_AUTO_SQUARIFY_NARROW_TEXTURE_SHAPE
so it is user's responsibility to detect MALI GPU.

Renamed to WEBGL_AUTO_SQUARIFY_NARROW_TEXTURE_SHAPE, as we are not sure about when Mali GPU's varying becomes correct. We could add it when we want to maintain this.

tfjs-backend-webgl/src/webgl_util.ts line 404 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

if the texture is a 0 shaped tensor, we should not change it.
Which means Math.min(...textureShape) === (isPacked ? 2 : 1)

Since we need to include both Math.min(...textureShape) === 1 and Math.min(...textureShape) ===2 for isPacked === true, I instead added one more condition here.

tfjs-backend-webgl/src/webgl_util.ts line 420 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

cache this boolean value, since it is checked in multiple places.

Since the input for isLongNarrowTex is different for each branch, maybe we could not cache it

… getVenderInfo

Linchenn

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

tfjs-backend-webgl/src/webgl_util.ts line 420 at r2 (raw file):

Previously, Linchenn wrote…

Since the input for isLongNarrowTex is different for each branch, maybe we could not cache it

Moves checking isLongNarrowTex to the end of the function. Then we do not need to add isLongNarrowTex to multiple places.

qjia7

LGTM with nits. Thanks.

qjia7 · 2022-09-01T02:38:54Z

tfjs-backend-webgl/src/webgl_util.ts

-    return [1, size];
+  let textureShape: [number, number];
+  if (logShape.length <= 1 && size <= maxTexSize &&
+      size <= maxSizeForNarrorTex) {


nit: size <= maxSizeForNarrorTex is not needed?

Done. Thanks!

qjia7 · 2022-09-01T02:40:23Z

tfjs-backend-webgl/src/webgl_util.ts

@@ -395,30 +403,42 @@ export function getTextureShapeFromLogicalShape(
  }

  let size = util.sizeFromShape(logShape);
-  if (logShape.length <= 1 && size <= maxTexSize) {
-    return [1, size];
+  let textureShape: [number, number];


nit: let textureShape = null. Then the last else {} can be removed?

pyu10055

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

shanumante-sc · 2022-09-01T15:35:01Z

tfjs-backend-webgl/src/webgl_util.ts

@@ -368,8 +368,16 @@ export function getShapeAs3D(shape: number[]): [number, number, number] {
 export function getTextureShapeFromLogicalShape(
    logShape: number[], isPacked = false): [number, number] {
  let maxTexSize = env().getNumber('WEBGL_MAX_TEXTURE_SIZE');
+  let maxSizeForNarrorTex =


Suggested change

let maxSizeForNarrorTex =

let maxSizeForNarrowTex =

(same everywhere else)

Thanks! Done.

Add WEBGL_MAX_TEXTURE_DIMENSION_FOR_1D_TEXTURE

a2cf0b4

Linchenn requested a review from pyu10055 August 31, 2022 01:19

Merge branch 'master' into getVenderInfo

52dc644

Linchenn requested a review from qjia7 August 31, 2022 01:22

qjia7 approved these changes Aug 31, 2022

View reviewed changes

Linchenn commented Aug 31, 2022

View reviewed changes

qjia7 reviewed Aug 31, 2022

View reviewed changes

pyu10055 requested changes Aug 31, 2022

View reviewed changes

Linchenn and others added 5 commits August 31, 2022 10:55

update flag

0c3ef56

Merge branch 'getVenderInfo' of https://github.com/Linchenn/tfjs into…

b3b87aa

… getVenderInfo

Merge branch 'master' into getVenderInfo

14e6ea7

Merge branch 'getVenderInfo' of https://github.com/Linchenn/tfjs into…

5e4d910

… getVenderInfo

add tests

1677032

Linchenn requested a review from pyu10055 August 31, 2022 18:38

Linchenn commented Aug 31, 2022

View reviewed changes

pyu10055 requested changes Aug 31, 2022

View reviewed changes

rename flag

45dce51

Linchenn requested a review from pyu10055 August 31, 2022 21:55

Linchenn commented Aug 31, 2022

View reviewed changes

Linchenn and others added 4 commits August 31, 2022 19:01

codestyle

dd211d1

Merge branch 'master' into getVenderInfo

be95513

polish

227b732

Merge branch 'getVenderInfo' of https://github.com/Linchenn/tfjs into…

5c26e34

… getVenderInfo

Linchenn commented Sep 1, 2022

View reviewed changes

Linchenn requested a review from qjia7 September 1, 2022 02:09

qjia7 approved these changes Sep 1, 2022

View reviewed changes

Linchenn added 2 commits August 31, 2022 20:10

Update webgl_util.ts

755ae07

Update webgl_util.ts

31dbf36

pyu10055 approved these changes Sep 1, 2022

View reviewed changes

shanumante-sc approved these changes Sep 1, 2022

View reviewed changes

Linchenn added 2 commits September 1, 2022 09:11

typo

2ba932c

fix

fbbe791

Linchenn merged commit 83d69c9 into tensorflow:master Sep 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WebGL backend] Add max 1D texture dimension flag #6808

[WebGL backend] Add max 1D texture dimension flag #6808

Linchenn commented Aug 31, 2022 •

edited

Loading

Linchenn commented Aug 31, 2022

qjia7 left a comment

Linchenn left a comment

qjia7 left a comment

pyu10055 left a comment

Linchenn left a comment

Linchenn left a comment

pyu10055 left a comment

Linchenn left a comment

Linchenn left a comment

qjia7 left a comment

qjia7 Sep 1, 2022

Linchenn Sep 1, 2022

qjia7 Sep 1, 2022

Linchenn Sep 1, 2022

pyu10055 left a comment

shanumante-sc Sep 1, 2022

Linchenn Sep 1, 2022

[WebGL backend] Add max 1D texture dimension flag #6808

[WebGL backend] Add max 1D texture dimension flag #6808

Conversation

Linchenn commented Aug 31, 2022 • edited Loading

Bug summary

Problem

Findings

Solution

Linchenn commented Aug 31, 2022

qjia7 left a comment

Choose a reason for hiding this comment

Linchenn left a comment

Choose a reason for hiding this comment

qjia7 left a comment

Choose a reason for hiding this comment

pyu10055 left a comment

Choose a reason for hiding this comment

Linchenn left a comment

Choose a reason for hiding this comment

Linchenn left a comment

Choose a reason for hiding this comment

pyu10055 left a comment

Choose a reason for hiding this comment

Linchenn left a comment

Choose a reason for hiding this comment

Linchenn left a comment

Choose a reason for hiding this comment

qjia7 left a comment

Choose a reason for hiding this comment

qjia7 Sep 1, 2022

Choose a reason for hiding this comment

Linchenn Sep 1, 2022

Choose a reason for hiding this comment

qjia7 Sep 1, 2022

Choose a reason for hiding this comment

Linchenn Sep 1, 2022

Choose a reason for hiding this comment

pyu10055 left a comment

Choose a reason for hiding this comment

shanumante-sc Sep 1, 2022

Choose a reason for hiding this comment

Linchenn Sep 1, 2022

Choose a reason for hiding this comment

Linchenn commented Aug 31, 2022 •

edited

Loading