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

Introduce MLContext.createContextAsync #285

Closed

Conversation

huningxin
Copy link
Contributor

@huningxin huningxin commented Aug 28, 2022

This PR also restricts the sync version of create context to worker.

Fix #272

This PR presents an alternative solution to #274 that follows the current naming convention by appending "Async" postfix to async method.

method async sync
create a context ML.createContextAsync ML.createContext
build a graph MLGraphBuilder.buildAsync MLGraphBuilder.build
compute a graph MLContext.computeAsync MLContext.compute

@wchao1115 @anssiko @dontcallmedom @RafaelCintron @yuhonglin @wacky6 , PTAL. Thanks!


Preview | Diff

This PR also restricts the sync version of create context to worker.

Fix webmachinelearning#272
@huningxin huningxin requested review from anssiko and wchao1115 and removed request for anssiko August 28, 2022 09:17
Copy link
Member

@anssiko anssiko left a comment

Choose a reason for hiding this comment

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

It looks like the WG is reaching a compromise on this naming issue and is converging to follow the WebGPU naming convention (for details, see #274 (comment)).

The WG spent some time discussing this issue, and we noted there was no strong precedent for sync/async method naming we could have leaned on.

This is a last call for all folks watching this repo to provide feedback if they have new information to add to this discussion. For context, see issue #272 and alternative design PR #274.

Thanks everyone for your contributions!

@wacky6
Copy link

wacky6 commented Aug 30, 2022

Looks like the WG doesn't have strong preferences on the names. I'll ask Chromium folks to see if people have preferences.

We probably should also bring this up with W3C TAG reviewers.

@anssiko
Copy link
Member

anssiko commented Aug 30, 2022

(Design discussion to continue in the issue #272.)

@huningxin
Copy link
Contributor Author

Per the Resolution #01 of WebML WG Teleconference – 3 November 2022 that is

RESOLUTION: Per TAG recommendation adopt x() and xSync() naming pattern for createContext(), build() and compute() methods.

I am going to close this PR.

Thanks all for your review and inputs.

@huningxin huningxin closed this Nov 4, 2022
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.

Support asynchronous context creation
4 participants