Allow tensors for graph constants. by bbernhar · Pull Request #830 · webmachinelearning/webnn

bbernhar · 2025-03-19T16:52:15Z

As noted in the issue, sharing weights between graphs is a effective way to enable reuse of device buffers, allowing web developers to manage weight data in graphs similarly to other input or output buffers.

@fdwr @huningxin @reillyeon

Based on the work in Chromium by Austin Sullivan.

Resolves #760

Preview | Diff

index.bs

bbernhar · 2025-03-20T17:24:50Z

Thanks @inexorabletash and @huningxin! Addressed comments.

index.bs

inexorabletash

Overall LGTM, just some annoying nitpicks. I ran the lint tool locally over the PR to avoid surprises later.

index.bs

anssiko · 2025-03-25T11:40:20Z

(FYI - I enabled the CI workflow for this PR and all the tests passed, including the newly added lint rules from #831)

index.bs

inexorabletash

LGTM! Thank you for your patience.

index.bs

fdwr

👍

index.bs

reillyeon · 2025-03-28T00:01:03Z

Thank you for the discussion here. I'm starting to come around to the idea of constness being just another property of an MLTensor like readable or writable and the debugging story is compelling. This does make me wonder as Dwayne did about whether it's worthwhile to have a separate interface name. After all, we have similar requirements on readTensor() and writeTensor() for the readable and writable properties without creating additional types.

index.bs

As noted in the issue, sharing weights between graphs is a effective way to enable reuse of device buffers, allowing web developers to manage weight data in graphs similarly to other input or output buffers. Resolves webmachinelearning#760

index.bs

fdwr

👍

index.bs

inexorabletash

One tiny nit

index.bs

bbernhar · 2025-04-21T21:23:46Z

@fdwr Feel free to merge when ready. Thanks reviewers for the help.

fdwr

Looks fine to me. Deferring to Ningxin for final review, approval, and merging.

huningxin

LGTM

SHA: d98e86d Reason: push, by huningxin Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

inexorabletash reviewed Mar 19, 2025

View reviewed changes

index.bs Outdated Show resolved Hide resolved

huningxin reviewed Mar 20, 2025

View reviewed changes

bbernhar force-pushed the constant_tensor branch from 05fba84 to a79b9c7 Compare March 20, 2025 16:45

inexorabletash reviewed Mar 20, 2025

View reviewed changes

index.bs Outdated Show resolved Hide resolved

inexorabletash reviewed Mar 20, 2025

View reviewed changes

index.bs Outdated Show resolved Hide resolved

bbernhar force-pushed the constant_tensor branch from cdd7279 to 5c83cc9 Compare March 20, 2025 20:50

inexorabletash suggested changes Mar 20, 2025

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Show resolved Hide resolved

huningxin reviewed Mar 21, 2025

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

inexorabletash mentioned this pull request Mar 21, 2025

Tooling: Use WebIDL2 parser when linting to identify dictionaries #831

Merged

bbernhar force-pushed the constant_tensor branch 2 times, most recently from 5dae5e3 to 673ac96 Compare March 21, 2025 23:12

inexorabletash reviewed Mar 25, 2025

View reviewed changes

index.bs Outdated Show resolved Hide resolved

bbernhar force-pushed the constant_tensor branch from 673ac96 to 85010ec Compare March 25, 2025 22:11

inexorabletash approved these changes Mar 25, 2025

View reviewed changes

reillyeon reviewed Mar 26, 2025

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

bbernhar force-pushed the constant_tensor branch from 85010ec to 583efa1 Compare March 26, 2025 17:40

fdwr approved these changes Mar 27, 2025

View reviewed changes

index.bs Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

huningxin reviewed Mar 31, 2025

View reviewed changes

index.bs Show resolved Hide resolved

index.bs Show resolved Hide resolved

bbernhar added 2 commits March 31, 2025 11:51

Allow tensors for graph constants.

673b9cd

As noted in the issue, sharing weights between graphs is a effective way to enable reuse of device buffers, allowing web developers to manage weight data in graphs similarly to other input or output buffers. Resolves webmachinelearning#760

Add [[constantTensor]] slot to check if destroyed before build.

18fb614

bbernhar force-pushed the constant_tensor branch from 583efa1 to 18fb614 Compare March 31, 2025 19:16

Change to constant MLTensor.

3b69c13

fdwr reviewed Apr 18, 2025

View reviewed changes

index.bs Show resolved Hide resolved

Blank line before dfn

4fccbdd

fdwr reviewed Apr 18, 2025

View reviewed changes

index.bs Outdated Show resolved Hide resolved

Comma before then

261c93b

fdwr approved these changes Apr 18, 2025

View reviewed changes

bbernhar requested review from huningxin, inexorabletash and reillyeon April 18, 2025 20:58

reillyeon approved these changes Apr 18, 2025

View reviewed changes

index.bs Outdated Show resolved Hide resolved

inexorabletash approved these changes Apr 18, 2025

View reviewed changes

index.bs Outdated Show resolved Hide resolved

bbernhar added 3 commits April 21, 2025 14:19

Added non-normative note for @huningxin.

7bd2ae3

fix nit by @reillyeon

63089ff

fix unit by @inexorabletash

8b972ff

fdwr approved these changes Apr 21, 2025

View reviewed changes

huningxin approved these changes Apr 22, 2025

View reviewed changes

huningxin merged commit d98e86d into webmachinelearning:main Apr 22, 2025
2 checks passed

github-actions bot added a commit that referenced this pull request Apr 22, 2025

Merge pull request #830 from bbernhar/constant_tensor

de65543

SHA: d98e86d Reason: push, by huningxin Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Conversation

bbernhar commented Mar 19, 2025 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bbernhar commented Mar 20, 2025

Uh oh!

Uh oh!

Uh oh!

inexorabletash left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anssiko commented Mar 25, 2025

Uh oh!

Uh oh!

inexorabletash left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fdwr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

reillyeon commented Mar 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fdwr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

inexorabletash left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bbernhar commented Apr 21, 2025

Uh oh!

fdwr left a comment

Choose a reason for hiding this comment

Uh oh!

huningxin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bbernhar commented Mar 19, 2025 •

edited by pr-preview bot

Loading