Expose dimRoundingMode attribute of pool op

[BruceDai]

Link to webmachinelearning/webnn-polyfill#135.

@lina128 PTAL, thanks.

---

This change is 

[lina128]

Reviewable status: 0 of 1 approvals obtained (waiting on @BruceDai)

---

tfjs-core/src/ops/avg_pool.ts, line 51 at r1 (raw file):

 * @param outputSizes The height and width of output shape:
 *     `[outHeight, outWidth]`.

The use case is not clear from this description. Can you elaborate the use case here?

[BruceDai]

The use case is not clear from this description. Can you elaborate the use case here?

Hi @lina128, as you known, WebNN-polyfill is a JavaScript implementation of the Web Neural Network API, WebNN-polyfill now uses tf.js to implement op functions. Recently pool2d operations of WebNN API enabled new roundingType and outputSizes options, the roundingType could map to dimRoundingMode, and here's WebNN API - pool2d's outputSizes description

outputSizes: a sequence of long of length 2. The sizes of the two spacial dimensions of the output tensor. 
When the output sizes are explicitly specified, the options.roundingType is ignored. 
If not specified, the output sizes are automatically computed.

In tf.js, the output shape of pool operations currently are automatically computed. This proposal of adding outputSizes attribute to specify output-height and output-width is for the use case of WebNN API - pool2d supporting outputSizes option. Thanks.

[pyu10055]

Thanks

Reviewable status: 0 of 1 approvals obtained (waiting on @BruceDai and @lina128)

---

tfjs-core/src/ops/avg_pool.ts, line 51 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

 * @param outputSizes The height and width of output shape:
 *     `[outHeight, outWidth]`.

The use case is not clear from this description. Can you elaborate the use case here?

might be good to copy the description of these two field from the webnn page
https://github.com/webmachinelearning/webnn/pull/208/files#diff-5e793325cd2bfc452e268a4aa2f02b4024dd9584bd1db3c2595f61f1ecf7b985R1604

---

tfjs-core/src/ops/avg_pool.ts, line 82 at r1 (raw file):

    if (typeof pad === 'string'){
      throw new Error(
          `dimRoundingMode ${dimRoundingMode} is set when using ${pad} pad`);

the error message is showing the has happened, but not explain why the dimRoundingMode can not be set with the pad value.

---

tfjs-core/src/ops/conv_util.ts, line 366 at r1 (raw file):

    for (const t of [undefined, 'floor', 'round', 'ceil']) {
      [outputRows, outputCols] =
          computeOutputShape2D(inShape, fieldSize, stride, zeroPad,

the check should be done if roundingMode === null, otherwise should verify if the otuputSize matches with the provided roundingModel value.

[BruceDai]

Thanks @pyu10055

the check should be done if roundingMode === null, otherwise should verify if the otuputSize matches with the provided roundingModel value.

I added the explicit log information of The dimRoundingMode is ignored when the output sizes are explicitly specified. Is it ok for not checking given roundingMode with given otuputSize?

@pyu10055 @lina128 Please take another look at the updated commit, thanks.

[huningxin]

Thanks for review @pyu10055 and @lina128 .

The output sizes of WebNN is to be compatible to native APIs that don't support setting dim rounding mode. As TF.js already supports dim rounding mode, I don't think it needs to support the output sizes. The WebNN-poyfill can translate the output sizes to TF.js dim rounding mode.

@BruceDai , if it makes sense, please focus on exposing dimRoundingMode of TF.js pool op. Thanks.

[BruceDai]

Thanks @huningxin.

Yes, it makes sense. I will update this pr, then ping @pyu10055 and @lina128 for review.

[BruceDai]

@pyu10055 @lina128 I updated commit according to @huningxin's comment, PTAL, thanks.

[pyu10055]

Reviewable status: 0 of 1 approvals obtained (waiting on @BruceDai and @lina128)

---

tfjs-core/src/ops/max_pool.ts, line 81 at r3 (raw file):

    if (typeof pad === 'string') {
      throw Error(
          `Error in maxPool: pad must be an integer when using `  +

this error message has been repeated through multiple ops, it would be good to refactor it into a help method and share across all ops.

---

tfjs-core/src/ops/max_pool_test.ts, line 114 at r3 (raw file):

  });

  it('x=[2,2,3] f=[2,2] s=3 p=1 default dimRoundingMode', () => {

please add failure test across all ops.

[BruceDai]

this error message has been repeated through multiple ops, it would be good to refactor it into a help method and share across all ops.
please add failure test across all ops.

Thanks @pyu10055

I've refactored pad checking when using dimRoundingMode. I'm ongoing to write test cases for those ops, new commit will be submitted later, thanks.

[BruceDai]

@pyu10055 @lina128 Please take a look again, thanks.

[pyu10055]

Great work, thanks!

Reviewed 15 of 20 files at r3, 25 of 25 files at r4, all commit messages.
Reviewable status: complete! 1 of 1 approvals obtained (waiting on @BruceDai and @lina128)

[BruceDai]

@lina128 I rebased code, PTAL, thanks.

[lina128]

LGTM, thanks!

Reviewable status: complete! 2 of 1 approvals obtained (waiting on @BruceDai and @lina128)