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

fix torch backend resize with pad_to_aspectio_ratio is set to True #20797

Merged
merged 9 commits into from
Jan 25, 2025

Conversation

sineeli
Copy link
Contributor

@sineeli sineeli commented Jan 22, 2025

The resize doesn't work as expected while setting flag pad_to_aspectio_ratio=True.

  1. Resize with JAX works as expected.
  2. Resize with Torch needs fix.
  3. Resize with Torch Fixed Code

@codecov-commenter
Copy link

codecov-commenter commented Jan 22, 2025

Codecov Report

Attention: Patch coverage is 67.85714% with 9 lines in your changes missing coverage. Please review.

Project coverage is 82.01%. Comparing base (5df8fb9) to head (41ecb92).
Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
keras/src/backend/numpy/image.py 66.66% 3 Missing and 3 partials ⚠️
keras/src/backend/torch/image.py 70.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master   #20797    +/-   ##
========================================
  Coverage   82.01%   82.01%            
========================================
  Files         557      558     +1     
  Lines       52014    52150   +136     
  Branches     8037     8062    +25     
========================================
+ Hits        42657    42771   +114     
- Misses       7403     7416    +13     
- Partials     1954     1963     +9     
Flag Coverage Δ
keras 81.83% <67.85%> (+<0.01%) ⬆️
keras-jax 64.26% <0.00%> (+0.04%) ⬆️
keras-numpy 58.99% <42.85%> (+0.04%) ⬆️
keras-openvino 29.86% <0.00%> (-0.04%) ⬇️
keras-tensorflow 64.81% <0.00%> (+0.02%) ⬆️
keras-torch 64.19% <25.00%> (+0.02%) ⬆️

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.

@fchollet
Copy link
Collaborator

Thanks for the PR -- can you add a simple unit test for this change?

@sineeli
Copy link
Contributor Author

sineeli commented Jan 24, 2025

Thanks for the PR -- can you add a simple unit test for this change?

Added a tests under image_test.py with pad_to_aspect_ratio=True

@fchollet
Copy link
Collaborator

Thanks a lot! Can you take a look at the resulting test failure with the numpy backend? Looks like a numerical mismatch, perhaps numpy had the same issue. https://github.com/keras-team/keras/actions/runs/12956346950/job/36142411678?pr=20797

@sineeli
Copy link
Contributor Author

sineeli commented Jan 24, 2025

Thanks a lot! Can you take a look at the resulting test failure with the numpy backend? Looks like a numerical mismatch, perhaps numpy had the same issue. https://github.com/keras-team/keras/actions/runs/12956346950/job/36142411678?pr=20797

Yup it seems like that, logic of numpy backend is also off just corrected it.

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.

Nice, thank you for the fix!

@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels Jan 25, 2025
@fchollet fchollet merged commit d0b27d0 into keras-team:master Jan 25, 2025
10 checks passed
@google-ml-butler google-ml-butler bot removed the ready to pull Ready to be merged into the codebase label Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants