Implement sparse image warp #252

kyleabeauchamp · 2019-05-18T23:53:33Z

As requested in #25.

The main reason this is WIP is that I had to skip a handful of tests that were incompatible with eager mode. I still need to wrap my head around some of the eager testing / gradient mechanics, so I might not be able to finish this myself.

WindQAQ

Hi Kyle, thanks for the contribution! I leave some comments below and hope them could help you.

tensorflow_addons/image/interpolate_spline.py

WindQAQ · 2019-05-25T08:51:42Z

tensorflow_addons/image/interpolate_spline_test.py

+                    self.assertAllClose(interp_val[0, :, 0],
+                                        target_interpolation)
+
+    def test_nd_linear_interpolation_unspecified_shape(self):


To use placeholder in the test case, we could test it with decorator test_utils.run_deprecated_v1.
Another way to test the shape of None at graph build time is #260 (comment).

WindQAQ · 2019-05-25T08:56:27Z

tensorflow_addons/image/interpolate_spline_test.py

+            _ = interpolate_spline(train_points_ph, train_values_ph_invalid,
+                                   query_points_ph, order, reg_weight)
+
+    def test_interpolation_gradient(self):


I wonder if tf.test.compute_gradient can help you. test_utils.run_deprecated_v1 could be a workaround though.

kyleabeauchamp · 2019-06-07T02:48:34Z

Thanks for the comments, I believe I have addressed all three. On the third comment, I ended up switching to use GradientTape, which seems like the most natural thing to do moving forward in TF2. However, let me know if I'm mistaken there...

tensorflow_addons/image/sparse_image_warp_test.py

seanpmorgan · 2019-06-07T03:06:48Z

@yhliang2018 Could you give this a review when time allows? Thanks.
cc @parth-p

yhliang2018 · 2019-06-07T20:48:45Z

@seanpmorgan Sure, I will take a look.

yhliang2018

Thanks for the PR!

yhliang2018 · 2019-06-28T23:36:07Z

tensorflow_addons/image/interpolate_spline.py

@@ -0,0 +1,303 @@
+# Copyright 2018 The TensorFlow Authors. All Rights Reserved.


yhliang2018 · 2019-06-28T23:39:13Z

tensorflow_addons/image/interpolate_spline_test.py

@@ -0,0 +1,365 @@
+# Copyright 2018 The TensorFlow Authors. All Rights Reserved.


yhliang2018 · 2019-06-28T23:41:23Z

tensorflow_addons/image/sparse_image_warp.py

@@ -0,0 +1,201 @@
+# Copyright 2018 The TensorFlow Authors. All Rights Reserved.


nit: same as above.

yhliang2018 · 2019-06-28T23:42:43Z

tensorflow_addons/image/interpolate_spline_test.py

+class _QuadraticPlusSinProblem1D(_InterpolationProblem):
+    """1D interpolation problem used for regression testing."""
+    DATA_DIM = 1
+    HARDCODED_QUERY_VALUES = {


Is it better to move all these test data to a util file, like test_util.py?

I guess my preference is to leave them where they are so that the the tests and the test constants are co-located...also that's how the code was in TF.

yhliang2018 · 2019-06-28T23:51:11Z

tensorflow_addons/image/interpolate_spline_test.py

+
+import tensorflow as tf
+import tensorflow.compat.v1 as tf1  # TODO: locate placeholder
+from tensorflow.python.training import momentum


It can be replaced as tf1.train.MomentumOptimizer, as we try to avoid direct import as much as possible.

Do you need to make addons as TF 2.0 as possible? If so, you may want to check optimizer v2: https://www.tensorflow.org/versions/r2.0/api_docs/python/tf/keras/optimizers/SGD

yhliang2018 · 2019-06-28T23:52:28Z

tensorflow_addons/image/sparse_image_warp_test.py

+            num_boundary_points=3)
+
+        loss = tf.reduce_mean(tf.abs(warped_image - image))
+        optimizer = momentum.MomentumOptimizer(0.001, 0.9)


tf1.train.MomentumOptimizer.

yhliang2018 · 2019-06-29T00:09:19Z

tensorflow_addons/image/sparse_image_warp.py

+      merged_control_point_flows: augmented set of control point flows
+    """
+
+    batch_size = tensor_shape.dimension_value(control_point_locations.shape[0])


Use tf.compat.dimension_value: https://github.com/tensorflow/tensorflow/blob/5fed792127f6b9ad34a7be5f6a2b17ce90c50edd/tensorflow/python/framework/tensor_shape.py#L98

kyleabeauchamp · 2019-06-29T06:23:30Z

OK I think I've addressed all the comments.

kyleabeauchamp · 2019-06-30T18:27:28Z

So I had to add scipy to the requirements. In previous commits, even in this PR, it was transitively included from the tensorflow install. However, it was missing in the latest couple of commits. For now, the easiest solution was to include it in the requirements.

kyleabeauchamp · 2019-07-18T05:13:55Z

OK, I resolved all conflicts again on this PR, so it should be all synced up for review/merge.

seanpmorgan

Thanks Kyle! LGTM I'm not in love with binary test images as part of the repo, but it's easier than downloading and verifying the hash.

kyleabeauchamp added 14 commits April 25, 2019 20:02

Add bzl file to control CXX11_ABI

45535c4

Merge remote-tracking branch 'upstream/master'

28de11f

Rename macros

89855b6

Add tf.sysconfig lookup

6568587

Merge remote-tracking branch 'upstream/master'

469419f

Add TODO note in configure

8294024

Fix

04eed3f

Merge remote-tracking branch 'upstream/master'

0de13ad

Merge remote-tracking branch 'upstream/master'

b0cdb2b

Merge remote-tracking branch 'upstream/master'

2da1b18

Merge remote-tracking branch 'upstream/master'

50e5af6

Add sparse_image_warp and interpolate_spline

5805f7c

Fix tests for sparse_image_warp

d4499fd

Fix comment

b64a8c1

kyleabeauchamp requested review from WindQAQ and facaiy as code owners May 18, 2019 23:53

googlebot added the cla: yes label May 18, 2019

WindQAQ reviewed May 25, 2019

View reviewed changes

kyleabeauchamp added 2 commits June 6, 2019 18:33

Merge remote-tracking branch 'upstream/master' into siw

6041d98

Address comments, switch gradient calculation to use GradientTape

cc752af

Forgot to unskip one last test

1158a90

seanpmorgan reviewed Jun 7, 2019

View reviewed changes

tensorflow_addons/image/sparse_image_warp_test.py Outdated Show resolved Hide resolved

Remove private functions from init

1c5330e

seanpmorgan added the image label Jun 11, 2019

yhliang2018 reviewed Jun 29, 2019

View reviewed changes

kyleabeauchamp added 2 commits June 28, 2019 22:59

Address reviewer comments

745e0ee

Merge remote-tracking branch 'upstream/master' into siw

7891ba4

One last review fix

d4c06a4

kyleabeauchamp requested a review from qlzh727 as a code owner June 29, 2019 06:20

Temporarily add scipy to requirements

afee506

kyleabeauchamp requested a review from a team as a code owner June 30, 2019 18:19

kyleabeauchamp mentioned this pull request Jul 7, 2019

Implement Image functions #25

Closed

12 tasks

kyleabeauchamp changed the title ~~[WIP] Implement sparse image warp~~ Implement sparse image warp Jul 18, 2019

kyleabeauchamp added 3 commits July 17, 2019 21:04

Fix merge conflicts

b1c26f8

Fix resolved merge issue

2163733

Fix lint

45510f6

seanpmorgan added the kokoro:force-run label Jul 22, 2019

kokoro-team removed the kokoro:force-run label Jul 22, 2019

seanpmorgan approved these changes Jul 23, 2019

View reviewed changes

seanpmorgan merged commit 6ec28e5 into tensorflow:master Jul 23, 2019

		@@ -0,0 +1,303 @@
		# Copyright 2018 The TensorFlow Authors. All Rights Reserved.

		@@ -0,0 +1,365 @@
		# Copyright 2018 The TensorFlow Authors. All Rights Reserved.

		@@ -0,0 +1,201 @@
		# Copyright 2018 The TensorFlow Authors. All Rights Reserved.

Implement sparse image warp #252

Implement sparse image warp #252

Uh oh!

Conversation

kyleabeauchamp commented May 18, 2019

Uh oh!

WindQAQ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WindQAQ May 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyleabeauchamp commented Jun 7, 2019

Uh oh!

Uh oh!

seanpmorgan commented Jun 7, 2019

Uh oh!

yhliang2018 commented Jun 7, 2019

Uh oh!

yhliang2018 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyleabeauchamp commented Jun 29, 2019

Uh oh!

kyleabeauchamp commented Jun 30, 2019

Uh oh!

kyleabeauchamp commented Jul 18, 2019

Uh oh!

seanpmorgan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

WindQAQ May 25, 2019 •

edited

Loading