Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#12151: Replace avg_pool2d with global_avg_pool2d #14330

Merged
merged 5 commits into from
Nov 25, 2024

Conversation

jerrysky3
Copy link
Contributor

@jerrysky3 jerrysky3 commented Oct 28, 2024

Ticket

#12151

Problem description

In ttnn currently there are global_avg_pool2d and avg_pool2d. Both of them do global average pooling (compute a single average of each channel). It seems that avg_pool2d is not a public API (couldn't be found on https://docs.tenstorrent.com/ttnn/latest/ttnn/ttnn) and most places in tt-metal use global_avg_pool2d

This change removes the avg_pool2d and replace all its remaining usages with global_avg_pool2d. This frees up the name ttnn.avg_pool2d for the implementation of the actual average pooling API with kernel size (#12151)

What's changed

  • Remove the export of ttnn.avg_pool2d (which is actually global average pool2d)
  • Replace the remaining usages of avg_pool2d in tt-metal with global_avg_pool2d

Checklist

@jerrysky3 jerrysky3 force-pushed the jerrysky3/remove-interim-avg-pool2d branch from a082788 to e674160 Compare October 28, 2024 23:56
@jerrysky3
Copy link
Contributor Author

Thanks for the review! Could someone help me merge the PR? It seems like I still have no permission to merge
image

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

tests/tt_eager/ops/test_average_pool.cpp Show resolved Hide resolved
@ayerofieiev-tt ayerofieiev-tt merged commit af73fe3 into main Nov 25, 2024
9 checks passed
@ayerofieiev-tt ayerofieiev-tt deleted the jerrysky3/remove-interim-avg-pool2d branch November 25, 2024 06:32
spoojaryTT pushed a commit that referenced this pull request Nov 25, 2024
### Ticket
#12151

### Problem description
In ttnn currently there are `global_avg_pool2d` and `avg_pool2d`. Both
of them do global average pooling (compute a single average of each
channel). It seems that `avg_pool2d` is not a public API (couldn't be
found on https://docs.tenstorrent.com/ttnn/latest/ttnn/ttnn) and most
places in tt-metal use `global_avg_pool2d`

This change removes the avg_pool2d and replace all its remaining usages
with `global_avg_pool2d`. This frees up the name `ttnn.avg_pool2d` for
the implementation of the actual average pooling API with kernel size
(#12151)

### What's changed
- Remove the export of `ttnn.avg_pool2d` (which is actually global
average pool2d)
- Replace the remaining usages of `avg_pool2d` in tt-metal with
`global_avg_pool2d`

### Checklist
- [x] Post commit CI passes
(https://github.com/tenstorrent/tt-metal/actions/runs/11546482848)
- [ ] Blackhole Post commit (if applicable)
- [ ] Model regression CI testing passes (if applicable)
- [ ] Device performance regression CI testing passes (if applicable)
- [x] New/Existing tests provide coverage for changes

Co-authored-by: Artem Yerofieiev <169092593+ayerofieiev-tt@users.noreply.github.com>
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.

5 participants