Skip to content

Conversation

@xhcao
Copy link
Contributor

@xhcao xhcao commented Dec 15, 2022

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


This change is Reviewable

@xhcao xhcao requested review from gyagp and qjia7 and removed request for gyagp December 15, 2022 05:19
@xhcao xhcao force-pushed the maxPoolWithArgmax branch from c94507d to b6ae59e Compare December 19, 2022 07:57
@xhcao
Copy link
Contributor Author

xhcao commented Dec 19, 2022

@gyagp please help to review it, there is a pending patch that depends on it.

@xhcao xhcao force-pushed the maxPoolWithArgmax branch from b6ae59e to bcc6b06 Compare December 19, 2022 08:19

this.shaderKey = `pool2D_${poolType}`;
if (computePositions) {
this.positionStr = flattenPositions ?
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, missed one nit: Please move positionStr to getUserCode.

this.computePositions ? `var maxValue = 0.0;
var maxValueFound = 0.0;
var maxPosition = 0;` :
''}
Copy link
Contributor

Choose a reason for hiding this comment

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

          ${
        this.computePositions ? `var maxValue = 0.0;
            var maxValueFound = 0.0;
            var maxPosition = 0;` :
                                ''}
          var resultValue = ${
        this.poolType === 'avg' ? '0.0' : '-1.0 / pow(10.0, -20.0)'};
          var count = 0.0;

->

          ${
        this.computePositions ? `var maxValue = 0.0;
            var maxValueFound = 0.0;
            var maxPosition = 0;` :
                                `
          var resultValue = ${
        this.poolType === 'avg' ? '0.0' : '-1.0 / pow(10.0, -20.0)'};
          var count = 0.0;
`}

@xhcao xhcao force-pushed the maxPoolWithArgmax branch from bcc6b06 to dde80a7 Compare January 3, 2023 03:06
Copy link

@gyagp gyagp left a comment

Choose a reason for hiding this comment

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

LGTM

@xhcao xhcao force-pushed the maxPoolWithArgmax branch from dde80a7 to be6f6b5 Compare January 10, 2023 01:06
@xhcao xhcao force-pushed the maxPoolWithArgmax branch from be6f6b5 to 412597b Compare January 10, 2023 01:28
@xhcao xhcao merged commit 70b98fc into tensorflow:master Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants