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

Update output_padding argument in convolution transpose layer #20737

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sonali-kumari1
Copy link

Update output_padding argument in convolution transpose layer. Fixes #19870 and #20408

@codecov-commenter
Copy link

codecov-commenter commented Jan 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 29.84%. Comparing base (ab3c8f5) to head (a8a124e).
Report is 4 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (ab3c8f5) and HEAD (a8a124e). Click for more details.

HEAD has 8 uploads less than BASE
Flag BASE (ab3c8f5) HEAD (a8a124e)
keras 5 1
keras-numpy 1 0
keras-torch 1 0
keras-tensorflow 1 0
keras-jax 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #20737       +/-   ##
===========================================
- Coverage   81.95%   29.84%   -52.12%     
===========================================
  Files         553      553               
  Lines       51446    51458       +12     
  Branches     7957     7961        +4     
===========================================
- Hits        42164    15356    -26808     
- Misses       7346    35542    +28196     
+ Partials     1936      560     -1376     
Flag Coverage Δ
keras 29.84% <ø> (-51.94%) ⬇️
keras-jax ?
keras-numpy ?
keras-openvino 29.84% <ø> (+<0.01%) ⬆️
keras-tensorflow ?
keras-torch ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Please add a unit test for this change.

@@ -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
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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Assigned Reviewer
4 participants