Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[webgl] Fix uninitialized zeros tensors, fix dtype upcasting. #4064

Merged
merged 41 commits into from
Oct 17, 2020

Conversation

annxingyuan
Copy link
Contributor

@annxingyuan annxingyuan commented Oct 13, 2020

This fixes the nightly build error.

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


This change is Reviewable

@google-cla google-cla bot added the cla: yes label Oct 13, 2020
@annxingyuan annxingyuan changed the title [webgl] Short circuit read. [webgl] Fix uninitialized zeros tensors, fix dtype upcasting. Oct 16, 2020
@annxingyuan annxingyuan marked this pull request as ready for review October 16, 2020 14:51
@annxingyuan annxingyuan self-assigned this Oct 16, 2020
Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

  if (dtype === 'bool') {
    const zerosTensorInfo = backend.makeTensorInfo(
        [], 'bool', util.getTypedArrayFromDType('bool', 1));

I think typed array allow zero length, why the length needs to be set to 1 here?

Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the nightly build!

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

Copy link
Contributor Author

@annxingyuan annxingyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

Previously, pyu10055 (Ping Yu) wrote…

I think typed array allow zero length, why the length needs to be set to 1 here?

We need to actually upload the value 0 to the GPU so that we can check whether the input equals 0 as part of casting to bool. The reason this only fails in nightly is because for PR's our threshold for uploading values as textures as opposed to uniforms is 0 (so everything is uploaded as a texture). If we print a zero length typed array to a texture (or even if we print null to a texture), it will be sampled as 0, so tests pass. However there is a difference between uploading 0 as a uniform, and uploading an empty typed array as a uniform.

@annxingyuan annxingyuan merged commit 9f2467d into master Oct 17, 2020
@annxingyuan annxingyuan deleted the test_ci branch October 17, 2020 02:33
@pyu10055
Copy link
Collaborator

pyu10055 commented Oct 18, 2020

The test of concat a large number of tensors is still timing out in the latest nightly build:

Chrome 77.0.3865 (Windows 10.0.0) concat3d webgl2 {"WEBGL_VERSION":2,"WEBGL_CPU_FORWARD":false,"WEBGL_SIZE_UPLOAD_UNIFORM":0} concat a large number of tensors, axis=1 FAILED
	Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.
	    at <Jasmine>
Chrome 77.0.3865 (Windows 10.0.0) SLOW 5.288 secs: concat3d webgl2 {"WEBGL_VERSION":2,"WEBGL_CPU_FORWARD":false,"WEBGL_SIZE_UPLOAD_UNIFORM":0} concat a large number of tensors, axis=1
Chrome 77.0.3865 (Windows 10.0.0) SLOW 5.288 secs: concat3d webgl2 {"WEBGL_VERSION":2,"WEBGL_CPU_FORWARD":false,"WEBGL_SIZE_UPLOAD_UNIFORM":0} concat a large number of tensors, axis=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants