-
Notifications
You must be signed in to change notification settings - Fork 49
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 Status of this document note for Candidate Recommendation #340
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.
I assume merging this depends on the decision on the timing of potentially merging #322?
index.bs
Outdated
Status Text: <p class="note" role="note"> | ||
Further implementation experience and user feedback is being gathered for the | ||
{{MLCommandEncoder}} interface that proposes to enable more efficient WebGPU | ||
interoperability and the related proposal to |
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'm not #322 is directly related to WebGPU interop, so I think that needs rephrasing.
Also, I know we've adopted the phrase "WebGPU interoperability" but I think "WebGPU integration" is a better description of what we're looking for feedback on.
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.
Thanks, s/interoperability/integration might be better and consistent with the charter text too.
I want to hear @wchao1115's proposal how to phrase this before updating this PR.
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.
Agreed with @dontcallmedom. I don't think #322 is related to WebGPU interop, or more specifically the MLCommandEncoder
. This text is a bit misleading.
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.
Would you prefer to exclude MLCommandEncoder
from this note entirely? Feel free to propose a better wording. We want this text to be clear.
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 note on MLCommandEncoder
should remain as we do need more data from implementation experience. We also anticipated that the WebGPU folks will have substantive feedback on it.
But it's a different topic from #322 and I think it's best to keep them separate to avoid confusing people.
re: #322, in my view it's a bit strange to ask for an implementation feedback on something that is proposed to be removed from the spec, rather than something to be added. The proposed removal of the internal GPU device would, by definition, require no implementation for it.
As a thought exercise, I think it seems far more intuitive to remove this feature from the spec, add it as an experimental feature, then ask the developers for their feedback (of implementing it).
But that's just a thought exercise to illustrate the point. I'm not proposing that we do that. I'd much prefer that we just remove the feature from the spec for good as in my mind it's a broken feature.
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 next iteration is up for your review. Please take a look and suggest further refinements as needed. It helps if we can be concise while not oversimplifying things so improvements in that regard are especially welcome.
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.
@wchao1115 PR updated with a more concise version of this note, PTAL!
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!
@dontcallmedom does the updated PR look good to you too? |
@anssiko can you review 8439546 before I start preparing the document for its CR publication, following w3c/transitions#491 (comment) |
@dontcallmedom I submitted one comment. (As an author of this PR I cannot approve it via GH, but you can consider this approved.) |
Sorry, not seeing it? (or not finding it) |
index.bs
Outdated
@@ -28,6 +28,13 @@ Status Text: <p class="note" role="note"> | |||
further improvements are expected to be reflected in revised Candidate | |||
Recommendation Drafts and Snaphots. | |||
</p> | |||
<p>Before requesting transition to <a href="https://www.w3.org/standards/types#PR">Proposed Recommendation</a>, the Working Group will seek to demonstrate that:</p> | |||
<ul> | |||
<li>the API is implementable on top of existing major platform APIs, such as Android Neural Networks API, Windows DirectML, and macOS/iOS Metal Performance Shaders and Basic Neural Network Subroutines;</li> |
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.
One excess whitespace in is implementable
.
The substance of this line is in itself consistent with the current charter scope.
Should any of these major platform APIs be superseded with or abstracted out by new platform APIs by their owners, I expect the WG to be able to use such new platform APIs for this demonstration. I'm not sure if this detail needs to be explicitly recorded in the exit criteria or whether it can be considered common sense. Any of these platform APIs could also be (in theory) deprecated at any time.
What should be allowed is to be able to use the most appropriate API available on each platform at the time of the PR transition. I expect the implementation-experience phase we're now entering to help inform this.
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.
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.
given the discussion in https://github.com/webmachinelearning/webnn/pull/362/files#r1145693621, should "macOS/iOS Metal Performance Shaders and Basic Neural Network Subroutines" be replaced with ML Compute?
I agree with you that the list can only be reasonably read as informative; but if we already know better, we should use the best of our knowledge.
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.
To my understanding the more recent ML Compute uses MPS and BNNS so it could be an alternative implementation path on Apple platforms. I’d prefer wording that allows Apple’s ML experts to use any of these platform APIs for their possible implementation and let that be considered another implementation in this context.
Reword and refine the text based on review feeedback
Make the text more concise to address review feedback
as discussed in w3c/transitions#491
SHA: 77f1aca Reason: push, by dontcallmedom Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Related #322
Preview | Diff