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 input/outputShape information to all KDocs of layers (#211) #460

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

dosier
Copy link
Contributor

@dosier dosier commented Sep 29, 2022

No description provided.

dosier added 30 commits June 14, 2021 21:21
� Conflicts:
�	api/src/main/kotlin/org/jetbrains/kotlinx/dl/api/inference/keras/ModelSaver.kt
- added emphasis
- removed redundant note comment
@juliabeliaeva juliabeliaeva self-requested a review September 29, 2022 13:49
@ermolenkodev ermolenkodev requested review from ermolenkodev and removed request for juliabeliaeva November 25, 2022 13:46
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.

Sorry for such a late review.
There are a few minor issues with the docs.
You make the following comment in many places:

Use the keyword argument 'input_shape'
 (tuple of integers, does not include the samples axis)
 when using this layer as the first layer in a model.

But currently, we only support Input to be the first layer of the network. Moreover, there is no argument named input_shape in the constructor anywhere in the code.

@@ -10,6 +10,11 @@ import org.tensorflow.Shape
/**
* Zero-padding layer for 1D input (e.g. audio).
* This layer can add zeros in the rows of the audio tensor
*
* __Input shape:__ 3D tensor with shape `(batch_size, axis_to_pad, features)`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

axis_to_pad is a somewhat misleading name because an axis usually refers to the dimension's index, but here it refers to the size of the dimension. Just dim would be ok.

@@ -10,6 +10,11 @@ import org.tensorflow.Shape
/**
* Zero-padding layer for 3D input (e.g. video).
* This layer can add zeros in the rows, cols and depth of a video tensor.
*
* __Input shape:__ 5D tensor with shape `(batch_size, first_axis_to_pad, second_axis_to_pad, third_axis_to_pad, depth)`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, maybe it would be better to replace axis with dim

@dosier
Copy link
Contributor Author

dosier commented Mar 2, 2023

Sorry for such a late review. There are a few minor issues with the docs. You make the following comment in many places:

Use the keyword argument 'input_shape'
 (tuple of integers, does not include the samples axis)
 when using this layer as the first layer in a model.

But currently, we only support Input to be the first layer of the network. Moreover, there is no argument named input_shape in the constructor anywhere in the code.

I see, thank you for the feedback! Also my apologies for the late reply, I have been quite busy.
Do u have any suggestions on what to replace that with? E.g. "Depends on the Input layer"'? Or can it be omitted?

Changed N-D to 2D since only 2D input is supported for now.
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