Fixed wrong result for l2Pool2d when sliding window having two valid elements#125
Merged
huningxin merged 2 commits intowebmachinelearning:mainfrom Apr 8, 2025
Merged
Conversation
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Apr 2, 2025
This issue was introduced by incorrect implementation of l2Pool2d operator for the case of sliding window having two valid elements in webnn-baseline project. I've submitted a fixing PR[1] there, then got the correct expected data for this CL. [1]: webmachinelearning/webnn-baseline#125 Bug: 383232123 Change-Id: Icdbc0859e642b832330ae309668444a000ff280d Cq-Include-Trybots: luci.chromium.try:win11-blink-rel, mac14.arm64-blink-rel, mac14-blink-rel, mac15.arm64-blink-rel, mac15-blink-rel, linux-blink-rel
fdwr
reviewed
Apr 4, 2025
| sumOfSquares = previousValue + currentValue * currentValue; | ||
| } | ||
|
|
||
| return (currentIndex === array.length - 1) ? Math.sqrt(sumOfSquares) : sumOfSquares; |
There was a problem hiding this comment.
Hmm, it's kinda complicated to figure this out, given the special cases for the first and last element. Looking at what you did for other similar cases like how you implemented reduceSumSquare and reduceLogSum, would it be simpler and more compositional to implement 2D Lebesgue pooling (y = (x1^p + x2^p + ... + xn^p) ^ (1/p)) as:
export function l2Pool2d(input, options = {}) {
const squaredInput = pow(input, new Scalar(2));
const pooledInput = pool2d(squaredInput, sumReducer, options);
return pow(pooledInput, new Scalar(0.5));
}export function reduceSumSquare(input, options = {}) {
return reduceSum(pow(input, new Scalar(2)), options);
}
export function reduceLogSum(input, options = {}) {
return log(reduceSum(input, options));
}
Contributor
Author
There was a problem hiding this comment.
Updated! Please take another look, thanks!
fdwr
approved these changes
Apr 7, 2025
fdwr
left a comment
There was a problem hiding this comment.
Yeah, much clearer (at least to me, and hopefully later generations of readers after us too 😉).
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Apr 7, 2025
This issue was introduced by incorrect implementation of l2Pool2d operator for the case of sliding window having two valid elements in webnn-baseline project. I've submitted a fixing PR[1] there, then got the correct expected data for this CL. [1]: webmachinelearning/webnn-baseline#125 Bug: 383232123 Change-Id: Icdbc0859e642b832330ae309668444a000ff280d Cq-Include-Trybots: luci.chromium.try:win11-blink-rel, mac14.arm64-blink-rel, mac14-blink-rel, mac15.arm64-blink-rel, mac15-blink-rel, linux-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6415990 Reviewed-by: Austin Sullivan <asully@chromium.org> Auto-Submit: Feng Dai <feng.dai@intel.com> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Feng Dai <feng.dai@intel.com> Cr-Commit-Position: refs/heads/main@{#1443301}
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Apr 7, 2025
This issue was introduced by incorrect implementation of l2Pool2d operator for the case of sliding window having two valid elements in webnn-baseline project. I've submitted a fixing PR[1] there, then got the correct expected data for this CL. [1]: webmachinelearning/webnn-baseline#125 Bug: 383232123 Change-Id: Icdbc0859e642b832330ae309668444a000ff280d Cq-Include-Trybots: luci.chromium.try:win11-blink-rel, mac14.arm64-blink-rel, mac14-blink-rel, mac15.arm64-blink-rel, mac15-blink-rel, linux-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6415990 Reviewed-by: Austin Sullivan <asully@chromium.org> Auto-Submit: Feng Dai <feng.dai@intel.com> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Feng Dai <feng.dai@intel.com> Cr-Commit-Position: refs/heads/main@{#1443301}
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Apr 7, 2025
This issue was introduced by incorrect implementation of l2Pool2d operator for the case of sliding window having two valid elements in webnn-baseline project. I've submitted a fixing PR[1] there, then got the correct expected data for this CL. [1]: webmachinelearning/webnn-baseline#125 Bug: 383232123 Change-Id: Icdbc0859e642b832330ae309668444a000ff280d Cq-Include-Trybots: luci.chromium.try:win11-blink-rel, mac14.arm64-blink-rel, mac14-blink-rel, mac15.arm64-blink-rel, mac15-blink-rel, linux-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6415990 Reviewed-by: Austin Sullivan <asully@chromium.org> Auto-Submit: Feng Dai <feng.dai@intel.com> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Feng Dai <feng.dai@intel.com> Cr-Commit-Position: refs/heads/main@{#1443301}
huningxin
approved these changes
Apr 8, 2025
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Apr 9, 2025
…tests, a=testonly Automatic update from web-platform-tests webnn: fixed wrong results for l2Pool2d tests This issue was introduced by incorrect implementation of l2Pool2d operator for the case of sliding window having two valid elements in webnn-baseline project. I've submitted a fixing PR[1] there, then got the correct expected data for this CL. [1]: webmachinelearning/webnn-baseline#125 Bug: 383232123 Change-Id: Icdbc0859e642b832330ae309668444a000ff280d Cq-Include-Trybots: luci.chromium.try:win11-blink-rel, mac14.arm64-blink-rel, mac14-blink-rel, mac15.arm64-blink-rel, mac15-blink-rel, linux-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6415990 Reviewed-by: Austin Sullivan <asully@chromium.org> Auto-Submit: Feng Dai <feng.dai@intel.com> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Feng Dai <feng.dai@intel.com> Cr-Commit-Position: refs/heads/main@{#1443301} -- wpt-commits: ecd6a697f24a5f3206fa2b599c0a083a6b52df5a wpt-pr: 51783
aosmond
pushed a commit
to aosmond/gecko
that referenced
this pull request
Apr 10, 2025
…tests, a=testonly Automatic update from web-platform-tests webnn: fixed wrong results for l2Pool2d tests This issue was introduced by incorrect implementation of l2Pool2d operator for the case of sliding window having two valid elements in webnn-baseline project. I've submitted a fixing PR[1] there, then got the correct expected data for this CL. [1]: webmachinelearning/webnn-baseline#125 Bug: 383232123 Change-Id: Icdbc0859e642b832330ae309668444a000ff280d Cq-Include-Trybots: luci.chromium.try:win11-blink-rel, mac14.arm64-blink-rel, mac14-blink-rel, mac15.arm64-blink-rel, mac15-blink-rel, linux-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6415990 Reviewed-by: Austin Sullivan <asully@chromium.org> Auto-Submit: Feng Dai <feng.dai@intel.com> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Feng Dai <feng.dai@intel.com> Cr-Commit-Position: refs/heads/main@{#1443301} -- wpt-commits: ecd6a697f24a5f3206fa2b599c0a083a6b52df5a wpt-pr: 51783
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Apr 16, 2025
…tests, a=testonly Automatic update from web-platform-tests webnn: fixed wrong results for l2Pool2d tests This issue was introduced by incorrect implementation of l2Pool2d operator for the case of sliding window having two valid elements in webnn-baseline project. I've submitted a fixing PR[1] there, then got the correct expected data for this CL. [1]: webmachinelearning/webnn-baseline#125 Bug: 383232123 Change-Id: Icdbc0859e642b832330ae309668444a000ff280d Cq-Include-Trybots: luci.chromium.try:win11-blink-rel, mac14.arm64-blink-rel, mac14-blink-rel, mac15.arm64-blink-rel, mac15-blink-rel, linux-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6415990 Reviewed-by: Austin Sullivan <asullychromium.org> Auto-Submit: Feng Dai <feng.daiintel.com> Reviewed-by: ningxin hu <ningxin.huintel.com> Commit-Queue: Feng Dai <feng.daiintel.com> Cr-Commit-Position: refs/heads/main{#1443301} -- wpt-commits: ecd6a697f24a5f3206fa2b599c0a083a6b52df5a wpt-pr: 51783 UltraBlame original commit: 85119108de0ce6ebcd38fb09b3db8ebc926841ee
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Apr 16, 2025
…tests, a=testonly Automatic update from web-platform-tests webnn: fixed wrong results for l2Pool2d tests This issue was introduced by incorrect implementation of l2Pool2d operator for the case of sliding window having two valid elements in webnn-baseline project. I've submitted a fixing PR[1] there, then got the correct expected data for this CL. [1]: webmachinelearning/webnn-baseline#125 Bug: 383232123 Change-Id: Icdbc0859e642b832330ae309668444a000ff280d Cq-Include-Trybots: luci.chromium.try:win11-blink-rel, mac14.arm64-blink-rel, mac14-blink-rel, mac15.arm64-blink-rel, mac15-blink-rel, linux-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6415990 Reviewed-by: Austin Sullivan <asullychromium.org> Auto-Submit: Feng Dai <feng.daiintel.com> Reviewed-by: ningxin hu <ningxin.huintel.com> Commit-Queue: Feng Dai <feng.daiintel.com> Cr-Commit-Position: refs/heads/main{#1443301} -- wpt-commits: ecd6a697f24a5f3206fa2b599c0a083a6b52df5a wpt-pr: 51783 UltraBlame original commit: 85119108de0ce6ebcd38fb09b3db8ebc926841ee
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Apr 16, 2025
…tests, a=testonly Automatic update from web-platform-tests webnn: fixed wrong results for l2Pool2d tests This issue was introduced by incorrect implementation of l2Pool2d operator for the case of sliding window having two valid elements in webnn-baseline project. I've submitted a fixing PR[1] there, then got the correct expected data for this CL. [1]: webmachinelearning/webnn-baseline#125 Bug: 383232123 Change-Id: Icdbc0859e642b832330ae309668444a000ff280d Cq-Include-Trybots: luci.chromium.try:win11-blink-rel, mac14.arm64-blink-rel, mac14-blink-rel, mac15.arm64-blink-rel, mac15-blink-rel, linux-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6415990 Reviewed-by: Austin Sullivan <asullychromium.org> Auto-Submit: Feng Dai <feng.daiintel.com> Reviewed-by: ningxin hu <ningxin.huintel.com> Commit-Queue: Feng Dai <feng.daiintel.com> Cr-Commit-Position: refs/heads/main{#1443301} -- wpt-commits: ecd6a697f24a5f3206fa2b599c0a083a6b52df5a wpt-pr: 51783 UltraBlame original commit: 85119108de0ce6ebcd38fb09b3db8ebc926841ee
jwidar
pushed a commit
to jwidar/LatencyZeroGithub
that referenced
this pull request
Sep 16, 2025
…tests, a=testonly Automatic update from web-platform-tests webnn: fixed wrong results for l2Pool2d tests This issue was introduced by incorrect implementation of l2Pool2d operator for the case of sliding window having two valid elements in webnn-baseline project. I've submitted a fixing PR[1] there, then got the correct expected data for this CL. [1]: webmachinelearning/webnn-baseline#125 Bug: 383232123 Change-Id: Icdbc0859e642b832330ae309668444a000ff280d Cq-Include-Trybots: luci.chromium.try:win11-blink-rel, mac14.arm64-blink-rel, mac14-blink-rel, mac15.arm64-blink-rel, mac15-blink-rel, linux-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6415990 Reviewed-by: Austin Sullivan <asully@chromium.org> Auto-Submit: Feng Dai <feng.dai@intel.com> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Feng Dai <feng.dai@intel.com> Cr-Commit-Position: refs/heads/main@{#1443301} -- wpt-commits: ecd6a697f24a5f3206fa2b599c0a083a6b52df5a wpt-pr: 51783
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is to fix the issue reported on https://issues.chromium.org/issues/383232123, the details of issue-383232123 as below.
@fdwr @huningxin PTAL, thanks!