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

Simplify api for building Layers #408

Merged
merged 7 commits into from
Aug 1, 2022

Conversation

juliabeliaeva
Copy link
Contributor

@juliabeliaeva juliabeliaeva commented Jul 26, 2022

This pull request attempts to simplify Layer building api by doing the following:

  1. Unify functional and sequential functions signatures.
  2. Remove separate output shape computation (btw, it was computed incorrectly in ZeroPadding1D and Concatenate layers).
  3. Combine Layer#build and Layer#forward functions.
  4. Combine GraphTrainableModel#build and GraphTrainableModel#forward functions.

This change was proposed as a part of Layer api simplification. Apart from the benefits described in linked proposal, having a single build method will make it easier to split Layer into an immutable Layer containing variables and layer output and a reusable LayerBuilder for building the Layer.
Possible downsides so far consists of one thing: the api no longer will mimic Keras api and it would be more difficult for contributors to borrow its code to implement new layers.

@juliabeliaeva juliabeliaeva added this to the 0.5 milestone Jul 26, 2022
Copy link
Collaborator

@ermolenkodev ermolenkodev left a comment

Choose a reason for hiding this comment

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

Looks great to me! Excellent that it's not needed to calculate output shape manually anymore.

@juliabeliaeva juliabeliaeva merged commit 579f167 into Kotlin:master Aug 1, 2022
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.

2 participants