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

split into sizes are not widely supported #392

Closed
huningxin opened this issue May 25, 2023 · 5 comments
Closed

split into sizes are not widely supported #392

huningxin opened this issue May 25, 2023 · 5 comments

Comments

@huningxin
Copy link
Contributor

This issue was raised by @wacky6 in Chromium CL review. Thanks Jiewei again!

WebNN split supports two variants, "number of split" (when splits argument is an unsigned long) or "split into sizes" (when splits argument is a sequence<unsigned long>).

When implementing this op, we observed that there are some backends don't support "split into sizes" variant, e.g. XNNPACK. For this case, we may follow the sample code that decomposes the split into multiple slices. Or just throw error and leave to framework to handle. There are tradeoffs, we'd like to get WG's inputs on:

If some backend doesn't offer the split implementation permitted in the spec, either 1) throw error, or 2) polyfill.

  1. doesn't look attractive since it can't be feature detected before building the graph. This also means split op isn't interoperable (because it depends on the underlying platform)
  2. implementation complexity (we need to bite the overhead if we slice())
@fdwr
Copy link
Collaborator

fdwr commented Sep 29, 2023

Although having a split operator in WebNN with variable size outputs is not technically not necessary (because it can be decomposed into multiple slice calls), it is rather common, and we really would want one for efficiency and for clear mapping from calling API's. The following all support variable length splits:

Options:

  1. Keep dedicated split with variable sizes - backends which support variable size splits can apply in-place copyless optimizations, avoid setting shader state several times, and bind only one input rather than the same input multiple times to different operators. Backends which do not support it could:
    1. emulate variable size splits - the backend can do this more efficiently than each individual calling library would achieve by repeating the logic, and it leaves room for the backend to support an optimized form later.
    2. throw an error - worst option imo, because it's quite awkward for the caller to handle. For clients like the ORT WebNN EP, it would be quite challenging to encounter such an error and then unwind and regenerate the graph again by splitting a split into multiple WebNN slice calls.
  2. Remove variable size split operator, forcing the front end to decompose variable lengths into slice calls (e.g. ORT's WebNN EP). The frontend loops with slice multiple calls.

Note, I've seen customer models that contain a split with over a hundred outputs, and needing the caller to output a hundred+ individual WebNN slice operators would be unfortunate.

@fdwr
Copy link
Collaborator

fdwr commented Oct 20, 2023

some backends don't support "split into sizes" variant

Does XNNPack support variable size windows for concat, which is splits' symmetric pair operator? (I just thought of this after reviewing the DML Concat CR)

@a-sully
Copy link
Contributor

a-sully commented Jun 3, 2024

FWIW the XNNPACK backend has since been removed from Chromium. All of the remaining backends currently being explored - CoreML, DML, and TFLite - support variable-sized split-by-size

Since any future backend we may want to implement WebNN on top of can emulate variable-sized splits if necessary, I say we close this issue. Thoughts?

@fdwr
Copy link
Collaborator

fdwr commented Jun 3, 2024

All of the remaining backends currently being explored - CoreML, DML, and TFLite - support variable-sized split-by-size

Great - vote to close then.

@huningxin
Copy link
Contributor Author

Closing, thanks for the update!

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

No branches or pull requests

5 participants