Skip to content

Fix: UpSampling2D bilinear set_image_data_format(channels_first) bug #21456

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

Conversation

tristandb8
Copy link
Contributor

The current approach transforms the tensor to channels_last, before passing it in ops.image.resize, which has been defined as channels_first if keras.backend.set_image_data_format has been called on channelsfirst in the user's code. This creates a bug, a passed in tensor [16, 3, 224, 224] will return as shape [16, 448, 224, 448] instead of [16, 3, 448, 448]. Setting the data_format as the expected channels_last fixes that issue.

I changed the unit tests to integrate keras.backend.set_image_data_format, also I noticed
layer = layers.UpSampling2D(
size=(length_row, length_col),
data_format=data_format,
)
Inside test_upsampling_2d_bilinear, wasn't using interpolation="bilinear", so I corrected that too

Closes #21401.

…assing it in ops.image.resize, which has been defined as channels_first if keras.backend.set_image_data_format has been called on channelsfirst in user's code. This creates a bug, a passed in tensor [16, 3, 224, 224] will return as shape [16, 448, 224, 448] instead of [16, 3, 448, 448]. Setting the data_format as the expected channels_last fixes that issue.
@codecov-commenter
Copy link

codecov-commenter commented Jul 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.88%. Comparing base (713172a) to head (76c5dcf).
Report is 30 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #21456      +/-   ##
==========================================
+ Coverage   82.72%   82.88%   +0.15%     
==========================================
  Files         565      565              
  Lines       55247    55705     +458     
  Branches     8613     8692      +79     
==========================================
+ Hits        45702    46169     +467     
+ Misses       7433     7421      -12     
- Partials     2112     2115       +3     
Flag Coverage Δ
keras 82.69% <100.00%> (+0.15%) ⬆️
keras-jax 63.47% <100.00%> (+0.12%) ⬆️
keras-numpy 58.66% <100.00%> (+0.15%) ⬆️
keras-openvino 33.99% <0.00%> (+0.21%) ⬆️
keras-tensorflow 63.89% <100.00%> (+0.14%) ⬆️
keras-torch 63.53% <100.00%> (+0.17%) ⬆️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gbaned gbaned added this to PR Queue Jul 7, 2025
@github-project-automation github-project-automation bot moved this to Assigned Reviewer in PR Queue Jul 7, 2025
@gbaned gbaned requested a review from amitsrivastava78 July 7, 2025 04:23
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!

num_samples = 2
stack_size = 2
input_num_row = 11
input_num_col = 12

if use_set_image_data_format:
set_image_data_format(data_format)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should not set this globally in a unit test, instead pass the argument when you create the UpSampling2D layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, that's the only way the error happens, as I've stated here
#21401 (comment)

There are no issues if I simply do
keras.layers.UpSampling2D(size=(2, 2), interpolation="bilinear", data_format="channels_first")(x)

I'm okay with just removing that line from the test, but it won't be testing my specific issue. Let me know if that's our best solution.

@divyashreepathihalli
Copy link
Collaborator

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request fixes a bug in UpSampling2D with bilinear interpolation when image_data_format is set globally to channels_first. The fix correctly forces data_format='channels_last' when calling ops.image.resize, as the input is always transposed to this format beforehand. The changes to the unit tests are good, especially adding coverage for the global setting and fixing the interpolation argument. Ensure test isolation by restoring the global image_data_format after the test runs to prevent potential side effects on other tests.

Comment on lines +79 to +80
if use_set_image_data_format:
set_image_data_format(data_format)
Copy link
Contributor

Choose a reason for hiding this comment

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

high

Modifying the global state image_data_format can lead to flaky tests. To ensure test isolation, save the original image_data_format before changing it and restore it after the test execution using a try...finally block.

        original_data_format = backend.image_data_format()
        if use_set_image_data_format:
            set_image_data_format(data_format)
        
        try:
            if data_format == "channels_first":
                inputs = np.random.rand(
                    num_samples, stack_size, input_num_row, input_num_col
                )
            else:
                inputs = np.random.rand(
                    num_samples, input_num_row, input_num_col, stack_size
                )

            self.run_layer_test(
                layers.UpSampling2D,
                init_kwargs={
                    "size": (2, 2),
                    "data_format": data_format,
                    "interpolation": "bilinear",
                },
                input_shape=inputs.shape,
            )

            layer = layers.UpSampling2D(
                size=(length_row, length_col),
                data_format=data_format,
                interpolation="bilinear",
            )
            layer.build(inputs.shape)
            np_output = layer(inputs=backend.Variable(inputs))
            if data_format == "channels_first":
                self.assertEqual(np_output.shape[2], length_row * input_num_row)
                self.assertEqual(np_output.shape[3], length_col * input_num_col)
            else:
                self.assertEqual(np_output.shape[1], length_row * input_num_row)
                self.assertEqual(np_output.shape[2], length_col * input_num_col)
        finally:
            if use_set_image_data_format:
                set_image_data_format(original_data_format)

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a good suggestion

@tristandb8
Copy link
Contributor Author

tristandb8 commented Jul 24, 2025

The last commit follows similar logic in
keras\src\applications\applications_test.py

@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels Jul 25, 2025
@github-project-automation github-project-automation bot moved this from Assigned Reviewer to Approved by Reviewer in PR Queue Jul 25, 2025
@fchollet fchollet merged commit 7b9ab6a into keras-team:master Jul 28, 2025
10 checks passed
@google-ml-butler google-ml-butler bot removed awaiting review ready to pull Ready to be merged into the codebase labels Jul 28, 2025
@github-project-automation github-project-automation bot moved this from Approved by Reviewer to Merged in PR Queue Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

keras.layers.UpSampling2D bilinear channel first bug
6 participants