webmachinelearning / webnn

🧠 Web Neural Network API
https://www.w3.org/TR/webnn/
Other
398 stars 48 forks source link

Define task source, construct with realms, improve parallel usage #593

Closed inexorabletash closed 9 months ago

inexorabletash commented 9 months ago

Relevant requirements/examples:


Preview | Diff

inexorabletash commented 9 months ago

@reillyeon @a-sully - can you take an initial look?

One thing I didn't touch that should probably be improved: "execute graph" still throws (DOMException, TypeError) which shouldn't happen in parallel, even though compute() takes that "throw" and turns it into a reject.

a-sully commented 9 months ago

One thing I didn't touch that should probably be improved: "execute graph" still throws (DOMException, TypeError) which shouldn't happen in parallel, even though compute() takes that "throw" and turns it into a reject.

FWIW the File System spec solves this by making the respective algorithm optionally return a DOMException error name and expecting the caller to queue a task to reject, as appropriate

inexorabletash commented 9 months ago

I have only one question which is whether it should be "ml task source" or "ML task source".

Capitalized "ml task" to "ML task" everywhere.

inexorabletash commented 9 months ago

One thing I didn't touch that should probably be improved: "execute graph" still throws (DOMException, TypeError) which shouldn't happen in parallel, even though compute() takes that "throw" and turns it into a reject.

FWIW the File System spec solves this by making the respective algorithm optionally return a DOMException error name and expecting the caller to queue a task to reject, as appropriate

I decided to tackle this and made a slight variation - the steps now return an error, and if an error is returned the invoker rejects with an equivalent error in the right realm. I think that's probably precise enough without being too wordy. I didn't just use an error name because TypeError and "OperationError" DOMException are both used.

a-sully commented 9 months ago

One thing I didn't touch that should probably be improved: "execute graph" still throws (DOMException, TypeError) which shouldn't happen in parallel, even though compute() takes that "throw" and turns it into a reject.

FWIW the File System spec solves this by making the respective algorithm optionally return a DOMException error name and expecting the caller to queue a task to reject, as appropriate

I decided to tackle this and made a slight variation - the steps now return an error, and if an error is returned the invoker rejects with an equivalent error in the right realm. I think that's probably precise enough without being too wordy. I didn't just use an error name because TypeError and "OperationError" DOMException are both used.

Seems reasonable to me 👍