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

webgpu: remove custom size parameter of dataToGPU #6393

Merged
merged 2 commits into from May 17, 2022

Conversation

xhcao
Copy link
Collaborator

@xhcao xhcao commented May 11, 2022

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


This change is Reviewable

@xhcao
Copy link
Collaborator Author

xhcao commented May 11, 2022

@qjia7 @haoyunfeix @axinging @gyagp, please take a look, thank you.

Copy link
Collaborator

@qjia7 qjia7 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

Add @lina128 to take another look.

@@ -381,6 +378,9 @@ export class Tensor<R extends Rank = Rank> {
* For WebGL backend, the data will be stored on a densely packed texture.
* This means that the texture will use the RGBA channels to store value.
*
* For WebGPU backend, the data will be stored on a buffer. There is no
* parameter, so can not use an user defined size to create the buffer.
*
* @param options:
* For WebGL,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lina128 Do you know why this part of description is not presented in https://js.tensorflow.org/api/latest/#tf.Tensor.dataToGPU Returns: GPUData? Is there a bug here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi, @lina128 , Please take a look, especially review the issue asked by Jiajia, thank you.

@qjia7 qjia7 requested a review from lina128 May 11, 2022 09:32
Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

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

LGTM

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


tfjs-core/src/tensor.ts line 385 at r2 (raw file):

https://js.tensorflow.org/api/latest/#tf.Tensor.dataToGPU
Good catch! I just realized the returns field is not captured at all, not just this one. We'll fix it. Thank you!

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

Successfully merging this pull request may close these issues.

None yet

3 participants