[webgpu] Update INT_DIV

[hujiajie]

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.

[qjia7]

LGTM, thanks!

[gyagp]

Several questions and comments:

1. round() in WGSL has a special definition: "Result is the integer k nearest to e, as a floating point value. When e lies halfway between integers k and k + 1, the result is k when k is even, and k + 1 when k is odd." Will this introduce a diff between CPU and WebGPU solution?
2. For "let remainder = round(numerator % denominator);" Is round() necessary?
3. What's the result of "9 / (-2)" in this code?
4. numerator -> dividend, denominator -> divisor?
5. Please ensure we have enough unit test cases for them, especially when dividend and/or divisor are halfway between integers, the results from "(numerator - remainder) / denominator" are halfway between integers, etc.

[hujiajie]

Several questions and comments:

1. round() in WGSL has a special definition: "Result is the integer k nearest to e, as a floating point value. When e lies halfway between integers k and k + 1, the result is k when k is even, and k + 1 when k is odd." Will this introduce a diff between CPU and WebGPU solution?

This is not clearly mentioned in the TensorFlow.js API doc. Even if the CPU rounding mode differs from the GPU backend, I cannot tell which one is more correct, or if either is fine.

2. For "let remainder = round(numerator % denominator);" Is round() necessary?

No, but I feel that's more aligned with neighboring lines.

3. What's the result of "9 / (-2)" in this code?

-5

4. numerator -> dividend, denominator -> divisor?

The API doc prefers numerator/denominator.

5. Please ensure we have enough unit test cases for them, especially when dividend and/or divisor are halfway between integers, the results from "(numerator - remainder) / denominator" are halfway between integers, etc.

[gyagp]

Different languages may have different results.
[python]
(-9) % 2 -> 1
9 % (-2) -> -1

[JavaScript]
(-9) % 2 -> -1
9 % (-2) -> 1

So your result will be at least different from the JS's result.
With round(), we introduce another level of difference between CPU and GPU, at least for the quotient calculation.

[hujiajie]

Please note INT_DIV is exclusively used by floorDiv(), and the doc states the result should be floored. This matches with the current CPU behavior (Math.floor(a / b)).

[qjia7]

JiaJie, if INT_DIV is only used for floorDiv, will it be better to rename INT_DIV to FLOOR_DIV?

[jiangzhaoming]

I have concern that the whole implementation of INT_DIV and floorDiv may conflict their expected semantic. The implementation, before and after this PR, is not coherent with floor(x/y), which is the definition of floorDiv(x, y). It only works correctly for integer input, and actually only tested on int32.
For example, input (5.9, 2) will result in 6.0 / 2.0 = 3.0 instead of floor(5.9 / 2.0) = floor(2.95) = 2.0, and input (3.7, 1.9) will result in 4.0 / 2.0 = 2.0 instead of floor(3.7 / 1.9) = floor(1.947368...) = 1.0. Input (5.4, 2.7) will result in 1.0 instead of 2.0.
I think the round of two inputs in original implementation are intend for handling integer inputs passing to WebGL as float, and we should revisit them.

[qjia7]

Good work, Jiajie. Please also add some test cases for float32 type in tfjs-core.

[gyagp]

I defer to Zhaoming's review on this.

[qjia7]

@jzm-intel Do you have any concern with the latest change? The test can be found here #7809.

[jiangzhaoming]

LGTM, thanks

[jiangzhaoming]

Why does it state as 'int32'? This is the last thing that confuse me....

[hujiajie]

This is the result dtype.

[jiangzhaoming]

Thanks, still LGTM.