-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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: export webgpu utilities #6707
Conversation
@haoyunfeix Please help to review it, thank you. |
} | ||
const device: GPUDevice = await adapter.requestDevice(deviceDescriptor); | ||
return new WebGPUBackend(device); | ||
}, 3 /*priority*/); | ||
} | ||
|
||
export {webgpu}; | ||
export * from './base'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not directly call export * from './webgpu';
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refer it to webgl backend, we could put all wanted export utilities in base.ts file in future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The webgl backend puts kernels into the register_all_kernels.ts
file and everything else into base.ts
. This is for better readability, and makes sure any side effects from setting up the backend happen before kernels are registered (I'm not 100% sure this is necessary). If you want to do the same for webgpu, you could move everything from index.ts into base.ts. However, it's a style choice, and if you prefer to keep everything in index.ts, I think that's fine.
tfjs-backend-webgpu/src/base.ts
Outdated
@@ -0,0 +1,19 @@ | |||
/** | |||
* @license | |||
* Copyright 2020 Google Inc. All Rights Reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2020 -> 2022
tfjs-backend-webgpu/src/index.ts
Outdated
@@ -49,11 +49,12 @@ if (isWebGPUSupported()) { | |||
}; | |||
|
|||
if (supportTimeQuery) { | |||
deviceDescriptor.requiredFeatures = ['timestamp-query' as const]; | |||
deviceDescriptor.requiredFeatures = ['timestamp-query' as const ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need an extra space here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had noticed this extra space, it was added automatically by vs editor.
ff23db2
to
c28e129
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.
This change is