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.

Sign up for GitHub

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

Update output_padding argument in convolution transpose layer #20737

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

Update output_padding argument in convolution transpose layer #20737

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8a124e
Update output_padding argument in convolution transpose layer
sonali-kumari1 Jan 8, 2025
9ba0806
Merge branch 'keras-team:master' into Patch1
sonali-kumari1 Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions keras/src/layers/convolutional/conv1d_transpose.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,22 @@ class Conv1DTranspose(BaseConvTranspose):
`"valid"` means no padding. `"same"` results in padding evenly to
the left/right or up/down of the input such that output has the same
height/width dimension as the input.
output_padding: An integer tuple/list of 1 integer specifying the
amount of padding along the time dimension of the output tensor.
The amount of output padding must be lower than the stride.
If set to `None` (default), the output shape is inferred.
data_format: string, either `"channels_last"` or `"channels_first"`.
The ordering of the dimensions in the inputs. `"channels_last"`
corresponds to inputs with shape `(batch, steps, features)`
while `"channels_first"` corresponds to inputs with shape
`(batch, features, steps)`. It defaults to the `image_data_format`
value found in your Keras config file at `~/.keras/keras.json`.
If you never set it, then it will be `"channels_last"`.
dilation_rate: int or tuple/list of 1 integers, specifying the dilation
rate to use for dilated transposed convolution.
dilation_rate: An integer tuple/list of 1 integer, specifying
the dilation rate to use for dilated convolution.
Currently, specifying a `dilation_rate` value != 1 is
incompatible with specifying a stride value != 1.
Also dilation rate larger than 1 is not currently supported.
activation: Activation function. If `None`, no activation is applied.
use_bias: bool, if `True`, bias will be added to the output.
kernel_initializer: Initializer for the convolution kernel. If `None`,
Expand Down Expand Up @@ -97,6 +104,7 @@ def __init__(
kernel_size,
strides=1,
padding="valid",
output_padding=None,
data_format=None,
dilation_rate=1,
activation=None,
Expand All @@ -116,6 +124,7 @@ def __init__(
kernel_size=kernel_size,
strides=strides,
padding=padding,
output_padding=None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be propagated, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @fchollet, When i am using output_padding=0 in keras(3.8.0), I am getting ValueError: The output_padding argument must be a tuple of 1 integers. Received output_padding=0, including values {0} that do not satisfy value > 0 but setting output_padding=None, it works without error. So, should I modify the error message to raise AttributeError or remove this argument ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @sonali-kumari1 ,

Which backend had this error? What was the trace for the exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @hertschuh ,

I have checked with tensorflow, torch and jax backends. I am getting the following traceback with all 3 backends:

[<ipython-input-6-1292b74f2ae3>](https://localhost:8080/#) in Model_HqTi1yjRFc7Dz1RLSJOvA9XX16R5Wp01(x)
     15     x = keras.activations.relu(x)
     16     _zeropadding_x = keras.layers.ZeroPadding1D(padding=(0, 0))(x)
---> 17     x = keras.layers.Conv1DTranspose(filters=128, kernel_size=1, strides=1, padding="valid", output_padding=0, data_format="channels_last", dilation_rate=1, use_bias=True, name="conv4_mutated")(x)
     18 
     19     x = x

[/usr/local/lib/python3.11/dist-packages/keras/src/layers/convolutional/conv1d_transpose.py](https://localhost:8080/#) in __init__(self, filters, kernel_size, strides, padding, data_format, dilation_rate, activation, use_bias, kernel_initializer, bias_initializer, kernel_regularizer, bias_regularizer, activity_regularizer, kernel_constraint, bias_constraint, **kwargs)
    111         **kwargs,
    112     ):
--> 113         super().__init__(
    114             rank=1,
    115             filters=filters,

[/usr/local/lib/python3.11/dist-packages/keras/src/layers/convolutional/base_conv_transpose.py](https://localhost:8080/#) in __init__(self, rank, filters, kernel_size, strides, padding, output_padding, data_format, dilation_rate, activation, use_bias, kernel_initializer, bias_initializer, kernel_regularizer, bias_regularizer, activity_regularizer, kernel_constraint, bias_constraint, trainable, name, **kwargs)
    109             self.output_padding = None
    110         else:
--> 111             self.output_padding = standardize_tuple(
    112                 output_padding,
    113                 rank,

[/usr/local/lib/python3.11/dist-packages/keras/src/utils/argument_validation.py](https://localhost:8080/#) in standardize_tuple(value, n, name, allow_zero)
     49             f" that do not satisfy `value {req_msg}`"
     50         )
---> 51         raise ValueError(error_msg)
     52 
     53     return value_tuple

ValueError: The `output_padding` argument must be a tuple of 1 integers. Received output_padding=0, including values {0} that do not satisfy `value > 0`

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok. Here:
https://github.com/keras-team/keras/blob/master/keras/src/layers/convolutional/base_conv_transpose.py#L115
You should add allow_zero=True, as an extra argument to standardize_tuple(), that's what's causing the error with a zero output_padding.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, the call chain looks something like this:
Conv1DTranspose (as initialized here) but applies to 2D and 3D as well → BaseConvTranspose processes the output_padding parameter using standardize_tuple() → Requires values to be greater than 0 (by default without the allow_zero = True parameter, but zero is a perfectly valid use case, and is even passed as the value to the output_padding parameter here, hence the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @hertschuh -

The output_padding argument is defined in the base class (BaseConvTranspose), but it is not mentioned in the Conv1DTranspose documentation. When setting output_padding=None in Conv1DTranspose, it works fine, possibly due to the inherited property from the base class (BaseConvTranspose).

However, when setting output_padding=0, a ValueError occurs. So, as suggested by you, I will add allow_zero=True, as an extra argument to standardize_tuple().

Can we update the Conv1DTranspose documentation to include the output_padding argument, as users typically follow the documentation?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The output_padding argument is defined in the base class (BaseConvTranspose), but it is not mentioned in the Conv1DTranspose documentation. When setting output_padding=None in Conv1DTranspose, it works fine, possibly due to the inherited property from the base class (BaseConvTranspose).

It is not inherited. It works fine because there is no output padding of you set output_padding=None.

However, when setting output_padding=0, a ValueError occurs. So, as suggested by you, I will add allow_zero=True, as an extra argument to standardize_tuple().

Yes, and then you can propagate ouput_padding in super().__init__(...).

Can we update the Conv1DTranspose documentation to include the output_padding argument, as users typically follow the documentation?

Yes, please update the documentation as you did here.

Thanks for updating the PR!

data_format=data_format,
dilation_rate=dilation_rate,
activation=activation,
Expand Down
19 changes: 17 additions & 2 deletions keras/src/layers/convolutional/conv2d_transpose.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ class Conv2DTranspose(BaseConvTranspose):
`"valid"` means no padding. `"same"` results in padding evenly to
the left/right or up/down of the input. When `padding="same"` and
`strides=1`, the output has the same size as the input.
output_padding: An integer or tuple/list of 2 integers,
specifying the amount of padding along the height and width
of the output tensor.
Can be a single integer to specify the same value for all
spatial dimensions.
The amount of output padding along a given dimension must be
lower than the stride along that same dimension.
If set to `None` (default), the output shape is inferred.
data_format: string, either `"channels_last"` or `"channels_first"`.
The ordering of the dimensions in the inputs. `"channels_last"`
corresponds to inputs with shape
Expand All @@ -38,8 +46,13 @@ class Conv2DTranspose(BaseConvTranspose):
`image_data_format` value found in your Keras config file at
`~/.keras/keras.json`. If you never set it, then it will be
`"channels_last"`.
dilation_rate: int or tuple/list of 1 integers, specifying the dilation
rate to use for dilated transposed convolution.
dilation_rate: An integer or tuple/list of 2 integers,
specifying the dilation rate for
all spatial dimensions for dilated convolution.
Specifying different dilation rates
for different dimensions is not supported.
Currently, specifying any `dilation_rate` value != 1 is
incompatible with specifying any stride value != 1.
activation: Activation function. If `None`, no activation is applied.
use_bias: bool, if `True`, bias will be added to the output.
kernel_initializer: Initializer for the convolution kernel. If `None`,
Expand Down Expand Up @@ -99,6 +112,7 @@ def __init__(
kernel_size,
strides=(1, 1),
padding="valid",
output_padding=None,
data_format=None,
dilation_rate=(1, 1),
activation=None,
Expand All @@ -118,6 +132,7 @@ def __init__(
kernel_size=kernel_size,
strides=strides,
padding=padding,
output_padding=None,
data_format=data_format,
dilation_rate=dilation_rate,
activation=activation,
Expand Down
18 changes: 16 additions & 2 deletions keras/src/layers/convolutional/conv3d_transpose.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ class Conv3DTranspose(BaseConvTranspose):
`"valid"` means no padding. `"same"` results in padding evenly to
the left/right or up/down of the input. When `padding="same"` and
`strides=1`, the output has the same size as the input.
output_padding: An integer or tuple/list of 3 integers,
specifying the amount of padding along the depth, height, and
width.
Can be a single integer to specify the same value for all
spatial dimensions.
The amount of output padding along a given dimension must be
lower than the stride along that same dimension.
If set to `None` (default), the output shape is inferred.
data_format: string, either `"channels_last"` or `"channels_first"`.
The ordering of the dimensions in the inputs. `"channels_last"`
corresponds to inputs with shape
Expand All @@ -38,8 +46,12 @@ class Conv3DTranspose(BaseConvTranspose):
It defaults to the `image_data_format` value found in your Keras
config file at `~/.keras/keras.json`. If you never set it, then it
will be `"channels_last"`.
dilation_rate: int or tuple/list of 1 integers, specifying the dilation
rate to use for dilated transposed convolution.
dilation_rate: an integer or tuple/list of 3 integers, specifying
the dilation rate to use for dilated convolution.
Can be a single integer to specify the same value for
all spatial dimensions.
Currently, specifying any `dilation_rate` value != 1 is
incompatible with specifying any stride value != 1.
activation: Activation function. If `None`, no activation is applied.
use_bias: bool, if `True`, bias will be added to the output.
kernel_initializer: Initializer for the convolution kernel. If `None`,
Expand Down Expand Up @@ -104,6 +116,7 @@ def __init__(
kernel_size,
strides=(1, 1, 1),
padding="valid",
output_padding=None,
data_format=None,
dilation_rate=(1, 1, 1),
activation=None,
Expand All @@ -123,6 +136,7 @@ def __init__(
kernel_size=kernel_size,
strides=strides,
padding=padding,
output_padding=None,
data_format=data_format,
dilation_rate=dilation_rate,
activation=activation,
Expand Down
Loading