Skip to content

Comments

[webnn] Add float32 tests for Element-wise logical operations#43304

Merged
Honry merged 2 commits intoweb-platform-tests:masterfrom
BruceDai:add_webnn_logical_tests
Nov 28, 2023
Merged

[webnn] Add float32 tests for Element-wise logical operations#43304
Honry merged 2 commits intoweb-platform-tests:masterfrom
BruceDai:add_webnn_logical_tests

Conversation

@BruceDai
Copy link
Contributor

@fdwr @Honry PTAL, thanks.

pow: {ULP: {float32: 32, float16: 2}},
// End Element-wise binary operations
// Begin Element-wise logical operations
equal: {ULP: {uint8: 0}},
Copy link
Contributor Author

@BruceDai BruceDai Nov 22, 2023

Choose a reason for hiding this comment

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

For logical operations, no mater input data type is float32 or float16 and etc. , the output type should be uint8.
And here uint8 is align with expected data type in json file, likes

"expected": {
  "name": "output",
  "shape": [2, 1, 4, 1, 3],
  "data": [
    0,
    0,
    0,
    1,
    0,
    ...
    0
  ],
  "type": "uint8"
}

We also use expected data type (precisionType) as a key to get tolerance:

// L#347
let tolerance = PrecisionMetrics[operationName][metricType][precisionType];

@fdwr Is it OK for this addition code? Thanks.

Copy link

Choose a reason for hiding this comment

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

👍

Copy link

@fdwr fdwr left a comment

Choose a reason for hiding this comment

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

👍

pow: {ULP: {float32: 32, float16: 2}},
// End Element-wise binary operations
// Begin Element-wise logical operations
equal: {ULP: {uint8: 0}},
Copy link

Choose a reason for hiding this comment

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

👍

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!

@BruceDai
Copy link
Contributor Author

Hi @fdwr, I synced with @huningxin, we decided to rename not op tests to previous logicalNot op tests, since Chromium WebNN API implementation has already supported logicalNot op, then these WPT logicalNot tests can be ran, please take another look at this update, thanks.

@fdwr
Copy link

fdwr commented Nov 28, 2023

@BruceDai : Ok, well whatever the final outcome of the spec is, it's a simple rename either way. 👍

@BruceDai BruceDai force-pushed the add_webnn_logical_tests branch from 75c0395 to 80d3ab8 Compare November 28, 2023 08:56
@BruceDai
Copy link
Contributor Author

Rebase code to check whether last two fail CIs can pass, then go to ask for merging it.

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