-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add MLTensor explainer #754
Conversation
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 with minor comments, thanks!
…hievable with identify
@bbernhar or @RafaelCintron hopefully none of this comes as a surprise? Any feedback? |
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!
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.
Well written, @a-sully. Minor nits + some comments.
mltensor-explainer.md
Outdated
|
||
For example [an `MLContext` may be created with a `GPUDevice`](https://www.w3.org/TR/webnn/#dom-ml-createcontext-gpudevice), and creating an `MLTensor` from this context with the `MLTensorUsage.WEBGPU_INTEROP` flag expresses a clear intention to share the tensor with the given `GPUDevice`. However, there is no guarantee that sharing this tensor with WebGPU will be zero-copy. | ||
|
||
The `MLTensorUsage.READ_FROM` and `MLTensorUsage.WRITE_TO` flags likewise are hints to the user agent indicating that the underlying data will be read and written to, respectively, by script. |
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.
Currently, specifying the correct MLTensorUsage
is a requirement (and not a hint). Perhaps in the future we can relax that.
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.
Ah, you're correct that usages are required from the JS API's perspective (we'll throw TypeError
if attempting to misuse an MLTensor
), but this text is speaking to user agents, not web developers. From the perspective of the user agent, it can allocate the buffer wherever it wants, but we're recommending it use MLTensorUsage
as a hint in that decision
I updated the first paragraph of this section to hopefully make this more clear. WDYT?
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 user agent still needs to figure out which device the buffer needs to be allocated on where omitting any MLTensorUsage
flag results could result into the buffer being left inaccessible (aka only used for output). Are you saying the user agent needs to either predict this and/or move/copy it around as needed?
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 believe the problem you're describing relates to this question? https://github.com/webmachinelearning/webnn/pull/754/files#diff-0ee55cff3e7c8d2280b48fa37962505a398c7a18df2eb78e0497c76042054cc8R286
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.
Sounds like the same problem. Also, if we eliminate deviceType
then we're back to using hints here so I suppose we need to think about how this will work.
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.
Correct, for the user agent it would stay "required" (for now).
just like how WebGPU can't force the system to use a "real" GPU (e.g. Warp exists)
AFAIK WebGPU doesn't allow a real GPU buffer to be created then used with a WARP device: a buffer must be created from the same device type, even if it happens to be CPU.
this explainer punts on finding a solution for this use case for now ¯(ツ)/¯
SGTM but currently, the explainer suggests this is possible as "hints" (ie. omitting flags == stays on deviceType
) which I think is too speculative.
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 don't want to haggle over this too much because this is an explainer and not a spec. As I mentioned above, the specification cannot dictate where/how the user agent allocates memory. When we specify MLTensor
, I expect we'll add non-normative guidance
I also wouldn't bank on deviceType
being around for much longer...
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.
As I mentioned above, the specification cannot dictate where/how the user agent allocates memory.
We could specify new hints which describe how frequently the tensor's data will change which could help (but not dictate) the user agent's decision where best to allocate it. Another option is we specify which device the allocation will start from. Giving web developers no control over these allocations doesn't seem ideal and we'll probably need to help them there, beyond what this explainer offers.
If you still prefer to address this later, I'm OK with that too.
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.
We could specify new hints which describe how frequently the tensor's data will change which could help (but not dictate) the user agent's decision where best to allocate it
Right, if it was useful to signal "this buffer will be used for WebGPU interop repeatedly and just read back to script once" or vice-versa, then the user agent may be able to make a more informed decision between the choices presented in #754 (comment):
- Allocate the
MLTensor
as a "default" buffer, requiring an additional staging buffer forreadBuffer()
, or- Allocate the
MLTensor
as a "readback" buffer and whip up a new GPU buffer for interop
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.
then the user agent may be able to make a more informed decision between the choices presented in #754 (comment)
The decision is only good as its last access and since inputs could be re-dispatched as outputs, I worry hints specified at creation could lead to bad choices being made repeatedly.
I think it's fine we have hints to determine what type of CPU allocation we start with (eg. read-back or upload buffer) but we'll also need to move it over to the GPU (eg. default buffer) then release the CPU allocation. If the web developer needs to WRITE
or READ
again then it'll move back to the CPU. If specified as such, web developers might have enough predictability to avoid staging buffers (and going OOM).
mltensor-explainer.md
Outdated
|
||
### Timelines | ||
|
||
WebNN uses a programming model similar to [WebGPU's](https://www.w3.org/TR/webgpu/#programming-model), in that compute tasks are posted to a timeline - which I've referred to as an "ML context timeline" throughout this document - separate from the content timeline (i.e. "script"). See [the WebGPU documentation of timelines](https://gpuweb.github.io/gpuweb/#programming-model-timelines) for more details. |
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.
nit: WebGPU folks, interested in WebNN, might want to know the "ML context timeline" here is equivalent to WebGPU's queue AND device timeline.
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.
Hmm we haven't actually specified WebNN's timelines yet (tracked in #529) so I'm hesitant to make such assertive statements for now
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 would say the example code already implies this and could be more clear:
// Rent out the MLTensor to WebGPU.
const tensorizedGpuBuffer = gpuDevice.importExternalBuffer(mlTensor1);
The MLTensor
being created by the MLContext
must be made available to the gpuDevice
which can be the same device also specified to createContext(), so they must be on an equivalent timeline.
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.
FYI #754 (comment) proposes making importExternalBuffer()
async, in part to decouple these timelines in the eyes of the spec
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 explainer now contains this note: https://github.com/webmachinelearning/webnn/pull/754/files#diff-0ee55cff3e7c8d2280b48fa37962505a398c7a18df2eb78e0497c76042054cc8R277
Is there more to discuss here or can we resolve this thread?
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.
Importing and returning the
MLTensor
are each points of synchronization between the respective WebNN and WebGPU timelines.
I have a slight preference to call out the device-queue timeline (separate from content/script timeline) since that is where we must control the execution of queue commands between WebNN and WebGPU. I'll leave it up to you if you want to comment it here or punt that for the real spec.
FYI @huningxin I'll be OOO next week so please feel free to merge this PR once @bbernhar is happy with the PR (please resolve comments which have been adequately addressed). I'm also happy to address some nits when I get back Thank you all for the thorough reviews! |
@a-sully thanks for producing this explainer and please enjoy your time off this week! We discussed this explainer a little on our call https://www.w3.org/2024/08/22-webmachinelearning-minutes.html#t04 and folks liked it. Reviewers should focus on the overall design, open questions. Spec changes including IDL will come in separate PRs. As @a-sully suggests, any IDL is tentative and is subject to change. webgpu interop issues addressed by the explainer can be marked for closure. @a-sully MLTensor (and broadly WebGPU interop) was proposed as one of the TPAC topics webmachinelearning/meetings#25 and I'd like you to introduce the proposal there. Features such as buffer-sharing between WebNN and WebGPU have cross-group interest. |
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.
Resolving addressed comments.
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.
@a-sully thank you very much for putting this together.
mltensor-explainer.md
Outdated
|
||
### Open Questions | ||
|
||
- How will errors be surfaced? Do we need a concept similar to [WebGPU's error scopes](https://www.w3.org/TR/webgpu/#error-scopes), or is [returning errors via a promise for select operations sufficient](https://github.com/webmachinelearning/webnn/issues/697#issuecomment-2195656878)? See [#477](https://github.com/webmachinelearning/webnn/issues/477) |
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.
WebGPU has error scopes because the API is very stateful and it's non-trivial to replicate all of the state checking in both the JS process and the GPU process. ErrorScopes can be challenging for web developers to use.
For WebNN, if implementations can efficiently check state in the JS process and throw exceptions on errors, we shouldn't need error scopes. For security, of course, the same state checking will also need to happen in the GPU process since a p000ned JS process can bypass error checking and send the GPU process whatever it wants.
In summary, in think we can leave ErrorScopes out of the spec for now and revisit if needed.
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 agree it would be nice to avoid the complexity of ErrorScopes until there's a demonstrated need. I've left it out from this proposal because there are bigger challenges to solve and I think we can mostly get away without a cohesive error-reporting mechanism, at least for now
My (longer-term) concern is that we don't have a reliable way to surface errors from dispatch()
. I don't think we can assume every failed dispatch()
results in a lost MLContext
, especially considering platforms where an MLContext
is not so closely tied to a single GPU
mltensor-explainer.md
Outdated
// For WebGPU Interop | ||
|
||
interface GPUExternalBuffer {}; | ||
GPUExternalBuffer includes GPUObjectBase; |
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.
Depending on how opaque the internal data is for WGSL shaders (see other comment) this wouldn't be an external buffer, but more like a GPUImportedMLTensor
maybe? Buffer makes it sound like the layout would be transparent and the buffer could be seen as an array<u32>
directly.
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.
See my response on the other comment. If this object is indeed addressable like a generic GPUBuffer
then is GPUExternalBuffer
reasonable or would you prefer the name indicate that it came from WebNN (by using "ML" or "tensor")?
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.
Updated to use array<T>
so we can just import as a GPUBuffer
. Longer-term, it would be nice to have some sort of GPUImportedTensor
type to abstract away the layout of the buffer, but we'll start with this
// Rent out the MLTensor to WebGPU. | ||
const tensorizedGpuBuffer = gpuDevice.importExternalBuffer(mlTensor1); | ||
|
||
// Create a bind group for `gpuVideoTexture`, create a command encoder, etc. |
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.
Can you describe a bit more how WebGPU would use tensorizedGpuBuffer
? Is it like a regular GPUBuffer
that can be addressed linearly, and the application would have some formula to know where to find individual tensor elements (this is much preferred) or would the tensor be an opaque type in WGSL that an application would have to address with some builtin function (workable, but likely to make it harder to write efficient algorithms)?
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.
Good question. Barring any unforeseen complications, I'm expecting the former: just a regular, linearly addressable GPUBuffer
In the short term, we can work around any complications by copying the tensor contents - which will be necessary in many cases regardless
@Kangz much thanks for your review and advice. We'll have a high-bandwidth discussion about this proposal at our TPAC F2F webmachinelearning/meetings#25 and your expertise would be of great use to the group in that session. You can join us in person or remotely (export your invite here). To narrow down on the timing, we'd get to this 23 Sep 2024 3-4pm PDT. |
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.
Some comments, but looks good. It's pretty clear with all the examples and scenarios. The variable names are pleasantly clear (e.g. imageAsMlTensor, imageAsArrayBuffer). Thank you for writing this.
mltensor-explainer.md
Outdated
|
||
// For WebGPU Interop | ||
|
||
interface GPUExternalBuffer {}; |
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.
What’s the benefit of having a new GPUExternalBuffer
object? It doesn’t seem like GPUExternalBuffer
represents something significantly different from a normal GPUBuffer
. I think we should consider if a new type is really necessary - because if we could get away without a new type and just overload/restrict GPUBuffer
it would save us the WebGPU and WGSL implementation work.
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.
@Kangz do you have any opinions on this? (related to #754 (comment) and #754 (comment))
1758c2f
to
f381d21
Compare
As agreed upon in discussions on the MLTensor explainer: webmachinelearning/webnn#754 Bug: 361372446 Cq-Include-Trybots: luci.chromium.try:win11-blink-rel Change-Id: I8bb993c1855c12eac68dc0f2ea359b5f1ae61932 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5858162 Commit-Queue: Austin Sullivan <asully@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: Alex Gough <ajgo@chromium.org> Cr-Commit-Position: refs/heads/main@{#1355445}
… MLTensorUsageFlags, a=testonly Automatic update from web-platform-tests webnn: Rename READ[_FROM] and WRITE[_TO] MLTensorUsageFlags As agreed upon in discussions on the MLTensor explainer: webmachinelearning/webnn#754 Bug: 361372446 Cq-Include-Trybots: luci.chromium.try:win11-blink-rel Change-Id: I8bb993c1855c12eac68dc0f2ea359b5f1ae61932 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5858162 Commit-Queue: Austin Sullivan <asullychromium.org> Reviewed-by: Reilly Grant <reillygchromium.org> Reviewed-by: Alex Gough <ajgochromium.org> Cr-Commit-Position: refs/heads/main{#1355445} -- wpt-commits: 31f51a67c7adbf4467cef8b0e70fe48b0141feda wpt-pr: 48173 UltraBlame original commit: 63277736fd23a46e332bedb1979fdf109564f7ce
mltensor-explainer.md
Outdated
|
||
Any `MLTensor` created with the `MLTensorDescriptor.importableToWebGPU` flag may be imported into any `GPUDevice`. In the best case, this requires no data copies. If the underlying buffer backing the `MLTensor` is not accessible to the `GPUDevice`, this will require copying the contents of the `MLTensor` to a new buffer, then copying the contents of this buffer back to the `MLTensor` once WebGPU releases its handle to the buffer. | ||
|
||
While an `MLTensor` is rented to a `GPUDevice`, the `GPUDevice` has exclusive, read/write access to the imported buffer, which is created as a `GPUExternalBuffer` with `GPUBufferUsageFlags.STORAGE`. All WebNN work depending - directly or indirectly - on the imported `MLTensor` is blocked until the `GPUDevice` returns the tensor. |
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.
Maybe add COPY_SRC/COPY_DST
for convenience as well? It should always be available without constraints.
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.
mltensor-explainer.md
Outdated
|
||
Any `MLTensor` created with the `MLTensorDescriptor.importableToWebGPU` flag may be imported into any `GPUDevice`. In the best case, this requires no data copies. If the underlying buffer backing the `MLTensor` is not accessible to the `GPUDevice`, this will require copying the contents of the `MLTensor` to a new buffer, then copying the contents of this buffer back to the `MLTensor` once WebGPU releases its handle to the buffer. | ||
|
||
While an `MLTensor` is rented to a `GPUDevice`, the `GPUDevice` has exclusive, read/write access to the imported buffer, which is created as a `GPUExternalBuffer` with `GPUBufferUsageFlags.STORAGE`. All WebNN work depending - directly or indirectly - on the imported `MLTensor` is blocked until the `GPUDevice` returns the tensor. |
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.
Sorry if this has been answered earlier: is the size and layout of the buffer defined somewhere such that the WGSL code manipulating the buffer knows where to find each element?
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.
Please correct me if I'm misunderstanding the question, but yes, I believe so. The size of the buffer is determined by its data type and shape. The layout is an implementation detail from the perspective of WebNN, but in practice the buffer will always be a type that GPU code (in the Chromium implementation, at least) understands. For example on Mac we'll import the buffer as an IOSurface
(which can be imported as a shared image)
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.
Ok I think we need more precision here. How would a user write a WGSL shader that manipulates a tensor when trying to for example implement custom operators? WebGPU sees storage buffers as a byte-addressed linear allocation, so the application needs to be aware of the layout of the tensor to address the allocation to find a specific element.
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.
Added a snippet showing the WGSL declaration of the imported buffer. See the diff in the latest patch: 4af9354
mltensor-explainer.md
Outdated
|
||
### Importing an `MLTensor` to WebGPU | ||
|
||
Any `MLTensor` created with the `MLTensorDescriptor.importableToWebGPU` flag may be imported into any `GPUDevice`. In the best case, this requires no data copies. If the underlying buffer backing the `MLTensor` is not accessible to the `GPUDevice`, this will require copying the contents of the `MLTensor` to a new buffer, then copying the contents of this buffer back to the `MLTensor` once WebGPU releases its handle to the buffer. |
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.
How would this handle cases where the device buffer strides are not aligned with the tensor dimensions? Or the buffer storage data type is not the same as the tensor data type, i.e. is packed with quantized valued (4 int8 within a int32).
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.
How would this handle cases where the device buffer strides are not aligned with the tensor dimensions?
Hmm I would assume WGSL only cares about buffer strides, so the tensor dimensions would be irrelevant to WebGPU. Is there a scenario you have in mind?
Or the buffer storage data type is not the same as the tensor data type,
Good question. Ideally the importableToWebGPU
flag can be used as a hint to allocate the tensor in an import-friendly fashion, but at the end of the day I expect we'll have to make a data copy in many scenarios, and possibly also accept that some MLTensor
s won't be importable to WebGPU at all. CoreML only supports creating an MLMultiArray
with a handful of data types, for example, and only float16 buffers can be imported to WebGPU without copies. If it's not float16, a copy will be required on Mac
FWIW WebNN does not currently support packing, since no operators operate on packed data. That being said, folks at Intel are working on adding a dequantizeLinear
op which aims to support "uint4"
which will presumably require packing since there's no Int4Array
in JS
As per the feedback on this thread on the MLTensor explainer PR: webmachinelearning/webnn#754 (comment) This CL includes logic to still support specifying the deprecated MLTensorUsage flags for now, though this logic will only exist for about a milestone to give callers the opportunity to migrate their existing code Bug: 343638938 Change-Id: I56209e68fde3920b8d6c781c8f804ac6fcd35c9a Cq-Include-Trybots: luci.chromium.try:mac14.arm64-blink-rel,win11-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5933323 Reviewed-by: ningxin hu <ningxin.hu@intel.com> Auto-Submit: Austin Sullivan <asully@chromium.org> Commit-Queue: ningxin hu <ningxin.hu@intel.com> Cr-Commit-Position: refs/heads/main@{#1370419}
As per the feedback on this thread on the MLTensor explainer PR: webmachinelearning/webnn#754 (comment) This CL includes logic to still support specifying the deprecated MLTensorUsage flags for now, though this logic will only exist for about a milestone to give callers the opportunity to migrate their existing code Bug: 343638938 Change-Id: I56209e68fde3920b8d6c781c8f804ac6fcd35c9a Cq-Include-Trybots: luci.chromium.try:mac14.arm64-blink-rel,win11-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5933323 Reviewed-by: ningxin hu <ningxin.hu@intel.com> Auto-Submit: Austin Sullivan <asully@chromium.org> Commit-Queue: ningxin hu <ningxin.hu@intel.com> Cr-Commit-Position: refs/heads/main@{#1370419}
As per the feedback on this thread on the MLTensor explainer PR: webmachinelearning/webnn#754 (comment) This CL includes logic to still support specifying the deprecated MLTensorUsage flags for now, though this logic will only exist for about a milestone to give callers the opportunity to migrate their existing code Bug: 343638938 Change-Id: I56209e68fde3920b8d6c781c8f804ac6fcd35c9a Cq-Include-Trybots: luci.chromium.try:mac14.arm64-blink-rel,win11-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5933323 Reviewed-by: ningxin hu <ningxin.hu@intel.com> Auto-Submit: Austin Sullivan <asully@chromium.org> Commit-Queue: ningxin hu <ningxin.hu@intel.com> Cr-Commit-Position: refs/heads/main@{#1370419}
…of boolean flags, a=testonly Automatic update from web-platform-tests webnn: Deprecate MLTensorUsage in favor of boolean flags As per the feedback on this thread on the MLTensor explainer PR: webmachinelearning/webnn#754 (comment) This CL includes logic to still support specifying the deprecated MLTensorUsage flags for now, though this logic will only exist for about a milestone to give callers the opportunity to migrate their existing code Bug: 343638938 Change-Id: I56209e68fde3920b8d6c781c8f804ac6fcd35c9a Cq-Include-Trybots: luci.chromium.try:mac14.arm64-blink-rel,win11-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5933323 Reviewed-by: ningxin hu <ningxin.hu@intel.com> Auto-Submit: Austin Sullivan <asully@chromium.org> Commit-Queue: ningxin hu <ningxin.hu@intel.com> Cr-Commit-Position: refs/heads/main@{#1370419} -- wpt-commits: ab3cd3a3748943c7ec96b7fdcc7b8e3acebe1396 wpt-pr: 48690
…of boolean flags, a=testonly Automatic update from web-platform-tests webnn: Deprecate MLTensorUsage in favor of boolean flags As per the feedback on this thread on the MLTensor explainer PR: webmachinelearning/webnn#754 (comment) This CL includes logic to still support specifying the deprecated MLTensorUsage flags for now, though this logic will only exist for about a milestone to give callers the opportunity to migrate their existing code Bug: 343638938 Change-Id: I56209e68fde3920b8d6c781c8f804ac6fcd35c9a Cq-Include-Trybots: luci.chromium.try:mac14.arm64-blink-rel,win11-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5933323 Reviewed-by: ningxin hu <ningxin.huintel.com> Auto-Submit: Austin Sullivan <asullychromium.org> Commit-Queue: ningxin hu <ningxin.huintel.com> Cr-Commit-Position: refs/heads/main{#1370419} -- wpt-commits: ab3cd3a3748943c7ec96b7fdcc7b8e3acebe1396 wpt-pr: 48690 UltraBlame original commit: 6f4fb11a34977957f99608befb8bd2ecaaec0153
…of boolean flags, a=testonly Automatic update from web-platform-tests webnn: Deprecate MLTensorUsage in favor of boolean flags As per the feedback on this thread on the MLTensor explainer PR: webmachinelearning/webnn#754 (comment) This CL includes logic to still support specifying the deprecated MLTensorUsage flags for now, though this logic will only exist for about a milestone to give callers the opportunity to migrate their existing code Bug: 343638938 Change-Id: I56209e68fde3920b8d6c781c8f804ac6fcd35c9a Cq-Include-Trybots: luci.chromium.try:mac14.arm64-blink-rel,win11-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5933323 Reviewed-by: ningxin hu <ningxin.huintel.com> Auto-Submit: Austin Sullivan <asullychromium.org> Commit-Queue: ningxin hu <ningxin.huintel.com> Cr-Commit-Position: refs/heads/main{#1370419} -- wpt-commits: ab3cd3a3748943c7ec96b7fdcc7b8e3acebe1396 wpt-pr: 48690 UltraBlame original commit: 6f4fb11a34977957f99608befb8bd2ecaaec0153
…of boolean flags, a=testonly Automatic update from web-platform-tests webnn: Deprecate MLTensorUsage in favor of boolean flags As per the feedback on this thread on the MLTensor explainer PR: webmachinelearning/webnn#754 (comment) This CL includes logic to still support specifying the deprecated MLTensorUsage flags for now, though this logic will only exist for about a milestone to give callers the opportunity to migrate their existing code Bug: 343638938 Change-Id: I56209e68fde3920b8d6c781c8f804ac6fcd35c9a Cq-Include-Trybots: luci.chromium.try:mac14.arm64-blink-rel,win11-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5933323 Reviewed-by: ningxin hu <ningxin.huintel.com> Auto-Submit: Austin Sullivan <asullychromium.org> Commit-Queue: ningxin hu <ningxin.huintel.com> Cr-Commit-Position: refs/heads/main{#1370419} -- wpt-commits: ab3cd3a3748943c7ec96b7fdcc7b8e3acebe1396 wpt-pr: 48690 UltraBlame original commit: 6f4fb11a34977957f99608befb8bd2ecaaec0153
…of boolean flags, a=testonly Automatic update from web-platform-tests webnn: Deprecate MLTensorUsage in favor of boolean flags As per the feedback on this thread on the MLTensor explainer PR: webmachinelearning/webnn#754 (comment) This CL includes logic to still support specifying the deprecated MLTensorUsage flags for now, though this logic will only exist for about a milestone to give callers the opportunity to migrate their existing code Bug: 343638938 Change-Id: I56209e68fde3920b8d6c781c8f804ac6fcd35c9a Cq-Include-Trybots: luci.chromium.try:mac14.arm64-blink-rel,win11-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5933323 Reviewed-by: ningxin hu <ningxin.hu@intel.com> Auto-Submit: Austin Sullivan <asully@chromium.org> Commit-Queue: ningxin hu <ningxin.hu@intel.com> Cr-Commit-Position: refs/heads/main@{#1370419} -- wpt-commits: ab3cd3a3748943c7ec96b7fdcc7b8e3acebe1396 wpt-pr: 48690
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.
WebGPU interop seems workable and LGTM
Thanks for the review! @huningxin or @fdwr would you mind merging this PR? If other reviewers have further feedback, I'm happy to address it in follow-up changes! |
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. Ningxin?
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!
This explainer builds off the discussions in #482, #541, and many others
Note that #753 proposes renaming
MLBuffer
toMLTensor
. I agree, and so this explainer reflects thatFeedback is welcome