Skip to content

Conversation

@BruceDai
Copy link
Contributor

}
},
"starts": [2, 2],
"sizes": [2, 4],
Copy link

@fdwr fdwr Dec 2, 2022

Choose a reason for hiding this comment

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

It's interesting that WebNN chose starts and sizes, whereas other frameworks I'm familiar with chose starts and ends, which makes it easier to specify relative endpoints on both ends. That will require some massaging from frameworks to WebNN. Shrug, pro's/con's. (no action expected)

Also the slice asymmetry between start and end ranges for negative values is weird 🤨, but that's a spec oddity (https://www.w3.org/TR/webnn/#api-mlgraphbuilder-slice), and the code looks fine as-is, according to the current version of the spec anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Some use ends/stop like ONNX (Slice) , OpenVINO (Slice), some use sizes/size, like DML (SLICE), TensorFlow (slice).

Copy link

@fdwr fdwr Dec 2, 2022

Choose a reason for hiding this comment

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

Plus:

fdwr
fdwr approved these changes Dec 2, 2022
@BruceDai BruceDai force-pushed the add_webnn_slice_tests branch from bce8fcd to 3da19a2 Compare January 6, 2023 07:26
@BruceDai
Copy link
Contributor Author

BruceDai commented Jan 6, 2023

@Honry All checks passed, PTAL, thanks.

Copy link
Contributor

@Honry Honry left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Honry Honry merged commit 062ac8b into web-platform-tests:master Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants