Skip to content
This repository was archived by the owner on Jul 1, 2023. It is now read-only.

Conversation

jekbradbury
Copy link
Contributor

For consistency with the other pooling APIs. Also updates to use squeezingAxes (should be NFC).

@jekbradbury jekbradbury requested a review from rxwei April 17, 2019 00:43
@Shashi456
Copy link
Contributor

@jekbradbury why is squeezing axes a better approach than the earlier one?

@rxwei
Copy link
Contributor

rxwei commented Apr 17, 2019

@jekbradbury why is squeezing axes a better approach than the earlier one?

It calls fewer TensorFlow ops. mean(squeezingAxis:) is equivalent to tf.reduce_mean(..., keep_dims=false). Though eventually we'd expect the compiler to optimize all of this away and make the performance be equivalent, the reality today is that the fewer op dispatches the better.

@rxwei rxwei merged commit 6f11c8e into tensorflow:master Apr 17, 2019
@Shashi456
Copy link
Contributor

Shashi456 commented Apr 17, 2019

@rxwei this wouldn't solve the non-differentiable error that pops up in #76 right?

@rxwei
Copy link
Contributor

rxwei commented Apr 17, 2019

The non-differentiable error there is because max(squeezingAxes:) does not have a derivative defined in the standard library.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants