-
Notifications
You must be signed in to change notification settings - Fork 58
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
Updated review of WebNN API #933
Comments
We've issued a CfC to advance with the CR Snapshot publication mid-March noting in this CfC the TAG delta review is currently in flight. We expect this issue to be looked at in the context of the transition request. As outlined in this issue, your earlier feedback (removal of sync APIs) has been addressed. The rest of the changes since your last review are evolutionary informed by implementation experience. Specifically, we are not expecting you to do another "full" review. If the group doesn't hear any concerns from you it plans to proceed with the publication. Thank you for your review comments (#771 (comment)) that motivated the removal of the sync APIs. |
Thanks @anssiko, @dontcallmedom for the review request. We were wondering, could you clarify the changes around transformers? We note you've added new data types and operations in support of them (is there a list?) - did you also add/remove any transformers? |
the list of operators added (and the removal of a redundant one) is in webmachinelearning/webnn#478 (comment) based on the detailed analysis made in webmachinelearning/webnn#375 (comment) |
@matatk you may also find the updated use cases for transformers webmachinelearning/webnn#507 helpful -- these use cases motivated the new ops discussed in the above-mentioned issue, also linked from the SOTD. To provide further context on the removal: one op (squeeze) was removed from the initial list of considered transformer ops because it was found out it can be expressed in terms of an existing lower-level op (reshape) in a performant manner. The emulation path for squeeze is presented informatively in the specification. Please let us know if you have any further questions. |
For full disclosure and to close the loop on this review: A new CR Snapshot (history) was published recently. Thank you for your questions and reviews (plural). We've already received two rounds of reviews from the TAG given we've hit the CRS milestone twice for this spec and appreciate your insights and persistence in working with us as we further evolve this specification. We look forward to another delta review with you as appropriate. If you have further review comments now or at any time do not hesitate to reach out to our group. We will consider all suggestions regardless of the spec milestone we're targeting. We're currently iterating on CRDs and plan to publish a new CRS approximately every 6-12 months. |
Hi @anssiko. Thank you for providing the context and info on recent changes, and for the publishing and cadence info. We are still looking into a few things on this review (noting that the 2024-04-29 version is now the current one, as you mentioned). We'll reply on this thread with any additional thoughts. |
We discussed WebNN earlier this week. We're generally happy with the way this is going. However, in previous discussions on this in the TAG, @cynthia expressed a concern regarding the threading approach - that it's possible that an intensive model running on the GPU could disrupt normal site/content rendering, and that would manifest as things like delays in |
@matatk and @cynthia with Chromium on Windows, the WebNN context runs on a separate command queue from the Chromium compositor. Depending on the device, the ML work may run on a separate chip than the one which performs 3D work. Even when it runs on the same chip, the ML work is multi-tasked with other work on the system. As with other web platform features (WebGL, WebGPU, 2D Canvas, CSS blurs, etc) overtaxing the system will eventually affect |
Hi @RafaelCintron - thanks for this detailed response. We're just discussing in our TAG breakout today. Can we just clarify 2 points:
|
@torgo, a challenge here is that WebNN supports more than just GPU compute. What @RafaelCintron mentioned makes sense as a concrete mitigation when the When using the CPU or a dedicated ML accelerator the types of potential resource contention and their mitigations are different. I think a general statement similar to WebGPU's reference to denial of service attacks makes sense to add to WebNN as well, with the understanding that exactly how the mitigations work will be implementation- and configuration-dependent. Implementations should use whatever mechanisms are available from the platform (such as the watchdogs mentioned by WebGPU) to prevent sites from using an unfair amount of system resources but in the end these are shared resources and the use of any compute API will affect overall performance on a fully-loaded system. |
Having some guidance in non-normative text, specifically around the different DoS vectors and mitigations would be helpful. |
Hi - thanks again for bringing this to us. We appreciate that you've been responsive to our feedback. We still have some concerns, but considering the current status of the work, we are planning to close the current review with a 'satisfied with concerns' label. Our main concern is: has this API considered the full range of hardware that it might need to run on? We see this running on CPUs without neural processing extensions, GPUs without extensions, CPUs with extensions, GPUs with extensions, and dedicated ML hardware. What steps have you taken to ensure that this runs across all of these targets, considering the range of hardware that exists and might exist? Our second and related concern is about multi-implementer support. If this is going to be ubiquitous as an approach to "do NN on the web" then it really needs to be implemented across different platforms and different hardware. We encourage you to consider these issues as the spec and technology continues to evolve. |
This is to address feedback from the TAG review: w3ctag/design-reviews#933
This is to address feedback from the TAG review: w3ctag/design-reviews#933 Co-authored-by: Dwayne Robinson <[email protected]>
We've made updates to the specification that we believe address the remaining concerns you had, namely:
Please let us know if there are any remaining concerns. Thank you! |
(extracted from #771 (comment))
I'm requesting an updated TAG review of WebNN API - previous TAG review: #771
Since the initial Candidate Recommendation Snapshot and the previous TAG review, the Working Group has gathered further implementation experience and added new operations and data types needed for well-known transformers webmachinelearning/webnn#375. In addition, the group has removed selected features informed by this implementation experience: higher-level operations that can be expressed in terms of lower-level primitives in a performant manner, and support for synchronous execution. The group has also updated the specification to use modern authoring conventions to improve interoperability and precision of normative definitions and is developing a new feature, a webmachinelearning/webnn#482, to improve performance and interoperability between the WebNN, WebGPU APIs and purpose-built hardware for ML.
The removal of support for synchronous execution is in-line with TAG's guidance (removal discussed in #531 and moving toward JSPI that is coming finally.
Further details:
You should also know that...
[please tell us anything you think is relevant to this review]
We'd prefer the TAG provide feedback as open issues in our GitHub repo for each point of feedback
The text was updated successfully, but these errors were encountered: