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

Add GlobalAvgPool1D Layer #74

Merged
merged 4 commits into from
Jun 2, 2021
Merged

Conversation

therealansh
Copy link
Contributor

@therealansh therealansh commented May 29, 2021

This PR adds GlobalAvgPool1D layer (#63). Also renamed the file to "GlobalAvgPooling2D" from "GlovalAvgPooling2d"(typo).

TODO:

  • Add layer implementation
  • Add support for export and import of layer in JSON format
  • Add Documentation
  • Add Tests

@zaleslaw zaleslaw linked an issue May 31, 2021 that may be closed by this pull request
@therealansh therealansh changed the title [WIP] Add GlobalAvgPool1D Layer Add GlobalAvgPool1D Layer May 31, 2021
@therealansh
Copy link
Contributor Author

The unit tests written right now are only for GlobalAvgPool1D extending the PoolingTest class. With the help of helper functions written by @avan1235 in the ShapeFunctions PoolLayerTest can be used as an open class for testing more Pooling layers in future.

Copy link
Collaborator

@zaleslaw zaleslaw left a comment

Choose a reason for hiding this comment

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

So, this LGTM for me also I'd like the tests, it's a great implementation and very clear for now
I suggest removing some blank lines in some places, but it's ready for merge.

There is a queue of PRs, I'll notify you when you should merge master into your branch (in a few days) and your PR will be merged

@zaleslaw zaleslaw added Review This PR is under review LGTM PR reviewed and is ready to merge and removed Review This PR is under review labels Jun 1, 2021
@zaleslaw
Copy link
Collaborator

zaleslaw commented Jun 1, 2021

So you could merge the master branch and I'll merge your PR in two days

@zaleslaw zaleslaw merged commit a672162 into Kotlin:master Jun 2, 2021
@zaleslaw
Copy link
Collaborator

zaleslaw commented Jun 2, 2021

Congrats, @therealansh thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LGTM PR reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Audio] Add GlobalAveragePooling1D layer
2 participants