Added the mode "bilinear" in the upscaling2D layer. #10994

gabrieldemarmiesse · 2018-08-25T14:26:20Z

Summary

Thanks @ahundt for your work. I was able to get Theano working and be compatible with what was doing tensorflow, but only for (2, 2) upscaling. Other factors like (4, 4) show different results for the theano's implementation and the tensorflow's implementation.
I also couldn't figure out how to use other factors like (3, 2) with theano. Which is why, with the Theano backend, for implementation and backend compatibility reasons, only (2, 2) is implemented.

Related Issues

See @ahundt 's PR: #9303

PR Overview

This PR requires new unit tests [y/n] (make sure tests are included)
This PR requires to update the documentation [y/n] (make sure the docs are up-to-date)
This PR is backwards compatible [y/n]
This PR changes the current API [y/n] (all API changes need to be approved by fchollet)

…sampling_bilinear

(2, 2) though.

ahundt

One small thing, otherwise looks good

ahundt · 2018-08-26T03:07:41Z

keras/backend/cntk_backend.py

    else:
-        raise ValueError('CNTK Backend: Invalid data_format:', data_format)
+        raise NotImplementedError


Isn't the existing behavior better here?

raise ValueError('CNTK Backend: Invalid data_format:', data_format)

We already have this message when giving as an input a wrong data_format. But it's true that the NotImplementedError is not clear at all. Let me change that.

fchollet

LGTM, thanks

fchollet · 2018-08-26T17:54:40Z

tests/keras/layers/convolutional_test.py

@@ -862,6 +862,45 @@ def test_upsampling_2d():
                assert_allclose(np_output, expected_out)


+@pytest.mark.skipif((K.backend() == 'cntk'),
+                    reason="cntk does not support it yet")


Use ' for string delimitation, for consistency

Of course, my bad.

joelthchao · 2018-08-27T05:51:47Z

tests/keras/backend/backend_test.py

+    @staticmethod
+    def _helper_bilinear(height_factor, width_factor):
+        x_shape = (2, 3, 4, 5)
+        for data_format in ['channels_first', 'channels_last']:


Suggest to make data_format as argument of _helper_bilinear. Then parameterize test_resize_images_bilinear with data_format. Otherwise, you may not be able to test both data_format with pytest.raises.

Good idea! I'll change that.

Dref360 · 2018-08-27T15:56:20Z

keras/backend/theano_backend.py

@@ -19,6 +19,7 @@
 except ImportError:
    from theano.sandbox.softsign import softsign as T_softsign

+import warnings


True. Let me remove that.

gabrieldemarmiesse · 2018-08-27T19:50:45Z

It seems that tests/test_multiprocessing.py::test_multiprocessing_predict_error is failing. I don't think it's related to this PR.

fchollet · 2018-08-28T17:21:13Z

tests/keras/layers/convolutional_test.py

@@ -862,6 +862,45 @@ def test_upsampling_2d():
                assert_allclose(np_output, expected_out)


+@pytest.mark.skipif((K.backend() == 'cntk'),


Please make this test parameterized (see #10975)

* keras/master: (327 commits) Added in_train_phase and in_test_phase in the numpy backend. (keras-team#11061) Make sure the data_format argument defaults to ‘chanels_last’ for all 1D sequence layers. Speed up backend tests (keras-team#11051) Skipped some duplicated tests. (keras-team#11049) Used decorators and WITH_NP to avoid tests duplication. (keras-team#11050) Cached the theano compilation directory. (keras-team#11048) Removing duplicated backend tests. (keras-team#11037) [P, RELNOTES] Conv2DTranspose supports dilation (keras-team#11029) Doc Change: Change in shape for CIFAR Datasets (keras-team#11043) Fix line too long in mnist_acgan (keras-team#11040) Enable using last incomplete minibatch (keras-team#8344) Better UX (keras-team#11039) Update lstm text generation example (keras-team#11038) fix a bug, load_weights doesn't return anything (keras-team#11031) Speeding up the tests by reducing the number of K.eval(). (keras-team#11036) [P] Expose monitor value getter for easier subclass (keras-team#11002) [RELNOTES] Added the mode "bilinear" in the upscaling2D layer. (keras-team#10994) Separate pooling test from convolutional test and parameterize test case (keras-team#10975) Fix issue with non-canonical TF version name format. Allow TB callback to display float values. ...

ahundt · 2018-09-27T02:56:23Z

cool! thanks for pushing this through @gabrieldemarmiesse!

gabrieldemarmiesse · 2018-09-27T07:01:12Z

You're welcome!

bonlime · 2018-10-16T16:30:26Z

@gabrieldemarmiesse
tf.image.resize_image has an align_corners parameter docs, which is impossble to pass using keras upsampling layer. But Theano doesn't have it. Is it possible to add support for this parameter for TF users?

SummitKwan · 2019-04-18T17:37:28Z

Hi thank you for the hard work! I have a question about the tensorflow implementation of resize_images.

The current implementation in keras uses tf.image.resize_images, which suffers from a list of problems depending on the version:

the bilinear interpolation have alignment problems: https://hackernoon.com/how-tensorflows-tf-image-resize-stole-60-days-of-my-life-aba5eb093f35
the tf.image.resize_images (for nearest neighbor) is not as efficient as tf.tile + tf.reshape, as is tested in Suggestion for efficient upsample+conv2d and conv2d+pool tensorflow/tensorflow#18711 (comment)
For 'channel_first' option in K.resize_images, the tensor has to go through 2 extra layers of transpose, it is going to slow down the operation?
It seems to me that tf.images.resize_images are designed for image pre-processing, not for intermediate layers (in early days, this op only runs on CPU, not GPU) are too generic for the simple goal of scaling the tensor by integer times, and tf.tile+tf.reshape sounds more reasonable for nearest, and tf.tile+tf.reshape+tf.conv2d (with pre-defined kernel) seems more reasonable for bilinear option.

In order not to break the previous code, would that make sense to add an option like 'nearest_by_tile', which uses the K.repeat_elements as in the CNTK back-end? Than you for your consideration and please correct me if I mis-understood things here

ahundt and others added 8 commits February 3, 2018 19:19

bilinear upsampling initial implementation

0855745

bilinear interpolation docstrings added

4b03e6a

thano_backend.py fix bilinear interpolation ratio

b7c10bd

theano_backend.py pep8 & padding fix

104f209

theano_backend.py remove border_mode as per review

73f0311

Merge branch 'bilinear_upsampling' of github.com:ahundt/keras into up…

dcc9dfb

…sampling_bilinear

Added the mode "bilinear" in the upscaling2D layer. Theano only supports

bd1b3b4

(2, 2) though.

Added some details about what was possible with bilinear interpolation.

2833ee4

gabrieldemarmiesse mentioned this pull request Aug 25, 2018

bilinear upsampling and resizing #9303

Closed

Made the lines shorted in the resize implementation of tensorflow.

8b6baf3

ahundt suggested changes Aug 26, 2018

View reviewed changes

Added a more meaningful error message for bilinear and CNTK.

c4b781e

fchollet previously approved these changes Aug 26, 2018

View reviewed changes

Used ' instead of " for consistency.

9d155e2

gabrieldemarmiesse dismissed fchollet’s stale review via 9d155e2 August 26, 2018 18:11

joelthchao reviewed Aug 27, 2018

View reviewed changes

Parametrized the bilinear test.

7770684

Dref360 reviewed Aug 27, 2018

View reviewed changes

Removed unused import.

2528906

Fix error messages.

70654da

fchollet reviewed Aug 28, 2018

View reviewed changes

Parametrized tests.

dac12e9

fchollet approved these changes Aug 30, 2018

View reviewed changes

fchollet merged commit cc3521e into keras-team:master Aug 30, 2018

gabrieldemarmiesse deleted the upsampling_bilinear branch September 18, 2018 06:34

ahundt mentioned this pull request Oct 4, 2018

Keras inference time optimizer keras-team/keras-contrib#264

Open

bhack mentioned this pull request Oct 7, 2018

Bilinear upsample bonlime/keras-deeplab-v3-plus#49

Closed

bhack mentioned this pull request Oct 17, 2018

[Feature Request] tf.keras expose align_corners tensorflow/tensorflow#23041

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added the mode "bilinear" in the upscaling2D layer. #10994

Added the mode "bilinear" in the upscaling2D layer. #10994

gabrieldemarmiesse commented Aug 25, 2018 •

edited

Loading

ahundt left a comment

ahundt Aug 26, 2018

gabrieldemarmiesse Aug 26, 2018

fchollet left a comment

fchollet Aug 26, 2018

gabrieldemarmiesse Aug 26, 2018

joelthchao Aug 27, 2018

gabrieldemarmiesse Aug 27, 2018

Dref360 Aug 27, 2018

gabrieldemarmiesse Aug 27, 2018

gabrieldemarmiesse commented Aug 27, 2018

fchollet Aug 28, 2018

ahundt commented Sep 27, 2018

gabrieldemarmiesse commented Sep 27, 2018

bonlime commented Oct 16, 2018 •

edited

Loading

SummitKwan commented Apr 18, 2019

		@@ -862,6 +862,45 @@ def test_upsampling_2d():
		assert_allclose(np_output, expected_out)


		@pytest.mark.skipif((K.backend() == 'cntk'),

Added the mode "bilinear" in the upscaling2D layer. #10994

Added the mode "bilinear" in the upscaling2D layer. #10994

Conversation

gabrieldemarmiesse commented Aug 25, 2018 • edited Loading

Summary

Related Issues

PR Overview

ahundt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fchollet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabrieldemarmiesse commented Aug 27, 2018

Choose a reason for hiding this comment

ahundt commented Sep 27, 2018

gabrieldemarmiesse commented Sep 27, 2018

bonlime commented Oct 16, 2018 • edited Loading

SummitKwan commented Apr 18, 2019

gabrieldemarmiesse commented Aug 25, 2018 •

edited

Loading

bonlime commented Oct 16, 2018 •

edited

Loading