Skip to content
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

Wording change: Streamline context creation and validation algorithms #519

Merged

Conversation

inexorabletash
Copy link
Member

@inexorabletash inexorabletash commented Jan 18, 2024

Bundle validation right into context creation, rather than making it a separate step.

Fixes #498


Preview | Diff

@inexorabletash
Copy link
Member Author

Note that there's still discussion in #498 (e.g. about the concept of context validity) so this is probably not a slam dunk to merge, but I wanted the artifact up to spur the discussion.

@zolkis
Copy link
Collaborator

zolkis commented Jan 18, 2024

Looks more compact, good.

@anssiko anssiko requested a review from huningxin January 18, 2024 19:48
index.bs Outdated Show resolved Hide resolved
@inexorabletash inexorabletash force-pushed the bugfix-context-creation branch from 3d25992 to f76ad2b Compare January 24, 2024 20:26
Copy link
Contributor

@huningxin huningxin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Bundle validation right into context creation, rather than making it
a separate step.

Note that with the removal of the "validate MLContext" algorithm, a
handful of the internal enum members are never referenced; they are
implicitly mapped to identical enums defined in Web IDL. This is
discussed in webmachinelearning#497 which proposes de-duping the enums. In the mean
time, the definitions are marked with "ignore" to silence warnings
about unreferenced defnitions.

Fixes webmachinelearning#498
@inexorabletash inexorabletash force-pushed the bugfix-context-creation branch from f76ad2b to 99ca098 Compare January 25, 2024 17:53
@inexorabletash
Copy link
Member Author

Rebased, should be ready to go now.

@wchao1115
Copy link
Collaborator

@inexorabletash Just checking if you can merge the change yourself, or whether you need me to click the button for you.

@inexorabletash
Copy link
Member Author

@inexorabletash Just checking if you can merge the change yourself, or whether you need me to click the button for you.

Nope, I can't merge - someone with "Write" permissions needs to do it. I don't even have "Triage" !

@inexorabletash
Copy link
Member Author

To be explicit, @wchao1115 - a click on this and any other "approved" PRs with no open discussion would be very appreciated.

@wchao1115 wchao1115 merged commit 1b76508 into webmachinelearning:main Jan 27, 2024
2 checks passed
github-actions bot added a commit that referenced this pull request Jan 27, 2024
…#519)

SHA: 1b76508
Reason: push, by wchao1115

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@inexorabletash inexorabletash deleted the bugfix-context-creation branch January 27, 2024 18:50
inexorabletash added a commit to inexorabletash/webnn that referenced this pull request Jan 30, 2024
…webmachinelearning#519)

Bundle validation right into context creation, rather than making it
a separate step.

Note that with the removal of the "validate MLContext" algorithm, a
handful of the internal enum members are never referenced; they are
implicitly mapped to identical enums defined in Web IDL. This is
discussed in webmachinelearning#497 which proposes de-duping the enums. In the mean
time, the definitions are marked with "ignore" to silence warnings
about unreferenced defnitions.

Fixes webmachinelearning#498
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Streamline context creation and validation algorithm definitions
5 participants