Conversation
huningxin
left a comment
huningxin
left a comment
There was a problem hiding this comment.
Thanks for the great work!
BruceDai
left a comment
BruceDai
left a comment
There was a problem hiding this comment.
Thanks @wchao1115 !
Here's a nit, PTAL.
|
Looks like the build isn't clean any more, these issues should be fixed: |
|
Here's current status for baseline implementation and WPT tests of new adding ops, PTAL, thanks.
|
This CL implements MLOperand.dataType() method according to proposal [1]. This method supports querying the operand data type at runtime. This implementation names the method dataType() that aligns with the MLOperandDescriptor.dataType [2]. [1]: webmachinelearning/webnn#478 [2]: https://www.w3.org/TR/webnn/#dom-mloperanddescriptor-datatype Bug: 1273291 Change-Id: I18c19d089e310b0c24e73dbb8c028fa351782082 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5056627 Reviewed-by: Jiewei Qian <[email protected]> Commit-Queue: ningxin hu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1228454}
@inexorabletash Fixed. |
@inexorabletash Fixed. |
+1. I think we should acknowledge Dwayne for his outstanding contribution. @wchao1115 @anssiko |
Fixed. 😊 |
|
All the discussions in this PR have been resolved, some comments spun into separate non-blocking issues as agreed with the reviewers, and the build is clean. Great work everyone involved! @wchao1115 @huningxin please take a final look. Given this is a big PR with many micro-commits, I'd like to hear if you'd prefer to squash this. Below is my reasoning and recommendation. For the earlier big PRs #446 we decided to keep the commit history in the main branch. For this PR here it looks like a squash merge might be preferred instead since there are many micro-commit that fix small issues and typos. GitHub will retain the commit history of this PR if we keep this PR around after the squash merge so references to these commits will continue to work. The only tradeoff I'm aware of is that the local git clones of this repo won't have access to the history of this PR by default. If you agree we want to squash this PR @wchao1115 please update your extensive PR summary in #478 (comment) to reflect the latest so we can reuse it as the squash commit message. This will help future contributors who will look at the history. Looking forward to merging this soon 🚀 |
10 participants
Issue #375.
equal, lesser, greater, greaterOrEqual, lesserOrEqual, not, where, copy, sqrt, erf, expand, gather, layerNormalization, reduceArgMin, reduceArgMax, cast, triangularsqueezeconstantoverload that acceptsstart, end, step.shapemethod inMLOperandfor runtime tensor shape query.int64anduint64forreduceArgMinandreduceArgMax.Notes:
identityop is namedcopyto denote explicit action.elementwiseIfis namedwhereto be consistent with PyTorch and ONNX.triangularMatrixis namedtriangularfor brevity.squeezeis removed. Framework seekingsqueeze,unsqueeze,flattenshould just usereshape. See canonical implementations of these ops asreshapein thereshapesection.layerNormalizationop is added. There are enough subtleties in the way affine parameters (e.g. scale, bias) are expected across batchNorm, instanceNorm, and layerNorm that could become ambiguous under the unified op. Additionally, downstream implementations of these normalizations have already been done in various platforms. The unification at the WebNN level could potentially confuse framework developers and affects currently defined platform-specific optimizations.fillSequenceop is added as aconstantoverload due to its nature of resource allocation activity.Big thanks to @fdwr for the thoughtful and thorough proposal.
Preview | Diff