Add support for tensorflow together with ODL #972

adler-j · 2017-04-13T16:45:34Z

TODO: summary

kohr-h · 2017-04-13T16:52:51Z

Phew.. My review here against your review on #861? :-P

adler-j · 2017-04-16T17:48:51Z

It's on its way :) But you wait until I rebase from master, this is actually quite a small PR.

kohr-h · 2017-05-18T22:39:21Z

Is this ready for review or should I wait?

adler-j · 2017-05-19T19:26:51Z

Still havn't merged from master, but you can easily get started if you want to.

kohr-h

I guess since this code is battle-tested there's not very much to say about the implementation itself. I just wonder if temporaries would make sense to speed up things, but that might quickly become infeasible. Dunno.

In general I find the wrapping code hard to digest, not because of complexity (it's really not that elaborate) but more because of all the inline function definitions that take up quite some space. If there's a way to make this a bit less nested, I'd appreciate it.

Anyway, docs could use some cleanup. Other than that it looks good (nice detective work on the ninja-style workarounds).

kohr-h · 2017-06-20T19:37:40Z

doc/source/guide/overview_functionality.rst

+
+
+What is vectorization?
+======================


Has this stuff moved here from some other file?

In any case, can you convert it to the "one sentence - one line" RST style?

kohr-h · 2017-06-20T19:38:42Z

doc/source/guide/overview_functionality.rst

+============
+
+Python functions are in most cases used as input to a discretization process. For example, we may
+want to discretize a two-dimensional Gaussian function:


Gaussian function ::

kohr-h · 2017-06-20T19:46:07Z

examples/contrib/tensorflow/tensorflow_layer_matrix.py

@@ -0,0 +1,38 @@
+"""Example of how to convert an ODL operator to a tensorflow layer."""
+
+import tensorflow as tf


Are we sure that odl.contrib.tensorflow is never confused with tensorflow (e.g. odl.contrib.tensorflow.tensorflow)?

If tensorflow wasn't already so long I'd suggest using a bindings suffix.. Maybe a shorter package name like tf_bindings will cut it?

I'm quite sure that is a nonproblem. With that said I'll change this whole thing to a submodule and split it, that allows better long-term extensions.

kohr-h · 2017-06-20T19:53:50Z

examples/contrib/tensorflow/tensorflow_layer_matrix.py

+
+# Create tensorflow layer from odl operator
+odl_op_layer = odl.contrib.tensorflow.as_tensorflow_layer(
+        odl_op, 'MatrixOperator')


indentation

kohr-h · 2017-06-20T19:55:36Z

examples/contrib/tensorflow/tensorflow_layer_matrix.py

+print(odl_op(x))
+
+# Evaluate the adjoint of the derivative, called gradient in tensorflow
+print(tf.gradients(y_tf, [x_tf], z_tf)[0].eval().ravel())


Perhaps use keyword arguments to make the code more self-explanatory. Now it's a bit of guesswork as to what variable stands for what.

kohr-h · 2017-06-20T22:06:40Z

odl/contrib/tensorflow.py

+        self.name = name
+
+    def _lincomb(self, a, x1, b, x2, out):
+        with tf.name_scope('{}_lincomb'.format(self.name)):


Will this clash if you use the same name twice?

No, it becomes _lincomb_2 etc

kohr-h · 2017-06-20T22:08:08Z

odl/contrib/tensorflow.py

+        return isinstance(other, TensorflowSpace) and other.shape == self.shape
+
+    def __repr__(self):
+        return 'TensorflowSpace({})'.format(self.shape)


Consider subclassers

kohr-h · 2017-06-20T22:08:34Z

odl/contrib/tensorflow.py

+        self._data = value
+
+    def __repr__(self):
+        return '{}.element({})'.format(self.space, self.data)


kohr-h · 2017-06-20T22:09:42Z

odl/contrib/tensorflow.py

+
+    """Wrap ODL operator so that it acts on TensorflowSpace elements."""
+
+    def __init__(self, domain, range, func, adjoint=None, linear=False):


Better use the same name for parameter and stored attribute.

kohr-h · 2017-06-20T22:11:42Z

odl/contrib/tensorflow.py

+    return tensorflow_layer
+
+
+class TensorflowSpace(LinearSpace):


A bit more docs for the fellas here and below wouldn't hurt.

kohr-h · 2017-06-20T22:16:07Z

And, to repeat one of the review comments, a top-level README is quite important for this stuff.

adler-j · 2017-06-21T03:57:48Z

Thanks for the review, I'll try to get this done and merged rather soon but I'm waiting for the FOM pull (and more free time for me) since I'll move some stuff into there.

adler-j · 2017-08-22T15:17:23Z

Fixed the comments, added some tests etc. IMO this can go in now.

kohr-h

Some final comments, merge when done.

Clearly a very cool addition!

kohr-h · 2017-08-23T21:58:28Z

odl/contrib/tensorflow/README.md

+
+## Example usage
+
+The [examples](examples) folder contains example on how to use the above functionality.


kohr-h · 2017-08-23T21:59:08Z

odl/contrib/tensorflow/README.md

+* [tensorflow_layer_matrix.py](examples/tensorflow_layer_matrix.py) shows how an ODL `MatrixOperator` can be converted to a tensorflow layer.
+* [tensorflow_layer_productspace.py](examples/tensorflow_layer_productspace.py) shows how an ODL operator acting on `ProductSpace`s can be converted to a tensorflow layer.
+* [tensorflow_layer_ray_transform.py](examples/tensorflow_layer_ray_transform.py) shows how a `RayTransform` can be converted to a tensorflow layer.
+* [tensorflow_operator_matrix.py](examples/tensorflow_operator_matrix.py) shows how `tf.matmul` can be used as a ODL operator.


kohr-h · 2017-08-23T22:02:09Z

odl/solvers/smooth/gradient.py

+
+        step = learning_rate * np.sqrt(1 - beta2) / (1 - beta1)
+
+        x.lincomb(1, x, -step, m / (np.sqrt(v) + eps))


Did you check this with the paper? Regarding your earlier answer to my review comment, yes, I read the paper, and I couldn't match the expressions there with the implementation here.

I'll review this once more then I guess!

kohr-h · 2017-08-23T22:04:20Z

odl/tomo/backends/astra_cuda.py

@@ -62,6 +63,10 @@ def __init__(self, geometry, reco_space, proj_space):

        self.create_ids()

+        # Create a mutually exclusive lock so that two callers cant use the
+        # same shared resource at the same time.
+        self.mutex = Lock()


bump (prefer this a bit more hidden, _mutex)

Agree, will fix that.

kohr-h · 2017-08-23T22:05:38Z

odl/util/numerics.py

@@ -802,6 +803,42 @@ def _apply_padding(lhs_arr, rhs_arr, offset, pad_mode, direction):
            working_slc[axis] = intersec_slc[axis]


+def zscore(arr):


Is that your naming or is it a standard notion?

https://se.mathworks.com/help/stats/zscore.html

Is the motivation.

There is also:

https://docs.scipy.org/doc/scipy-0.14.0/reference/generated/scipy.stats.mstats.zscore.html

But it does not handle constants "well".

adler-j · 2017-08-29T17:18:53Z

Merge after CI

adler-j added status: review needed type: new feature labels Apr 13, 2017

adler-j requested a review from kohr-h April 13, 2017 16:45

adler-j mentioned this pull request May 23, 2017

asarray() for power spaces #83

Closed

kohr-h requested changes Jun 20, 2017

View reviewed changes

kohr-h changed the title ~~Add support for tensorflow togeather with ODL~~ Add support for tensorflow together with ODL Jun 21, 2017

adler-j mentioned this pull request Jul 3, 2017

Release 1.0.0 #1055

Open

20 tasks

adler-j added 17 commits August 1, 2017 10:16

ENH: Add (initial) support for tensorflow interconnectivity

3387656

ENH: Add more tensorflow examples

3ab822b

ENH: Improve handling of shapes of tensorflow layers

83c27e4

ENH: Add more tensorflow examples

b4b4a20

ENH: Optimizations to the tensorflow interface

8d500aa

ENH: Improved CNN gradient tomography

faef60f

ENH: Support variable size input for tensorflow

c28586f

ENH: Improved cnn gradient example

8b0efdd

ENH: Improvements to the CNN tensorflow example

428087c

BUG: Fix bug with derivative of tensorflow operators

787eb99

ENH: Allow non-differentialbe operators in tensorflow

4ee1f9b

ENH: Add odl.contrib for less-than-optimal code

7a3a86b

ENH: Add TensorflowSpace

8817c05

BUG: Fix bug with default LinearSpace.zeros

96ed70c

ENH: Improvements to tensorflow support

8a61714

ENH: Minor performance optimization to douglas rachford solver

7977ff0

ENH: Add name and name scopes to tf space

2e7cc2a

adler-j added 8 commits August 1, 2017 12:12

MAINT: Cleanup tensorflow examples

c9ef813

API: Change shape of productspace and add asarray

1fe5ab2

ENH: Add estimate_noise

ba647fd

ENH: Add option to normalize PSNR

188d9dd

ENH: Add ADAM solver, see odlgroup#984

34f11fc

MAINT: Clean up tensorflow package structure

7722357

MAINT: fix bugs with tensorflow

f4028b8

MAINT: Bug fixes for product space

b563410

adler-j force-pushed the tensorflow_support branch from 77d83d0 to b563410 Compare August 1, 2017 12:08

ENH: Add TensorflowOperator

a25a575

adler-j mentioned this pull request Aug 3, 2017

module 'odl' has no attribute 'uniform_discr' #1083

Closed

adler-j added 11 commits August 3, 2017 15:46

MAINT: Improve tensorflow examples

8809225

DOC: Remove overview_functionality stub

e6db7e0

MAINT: Doc improvements to adam solver

c397833

MAINT: Improve doc of ProductSpace.element.__array__

6d06488

ENH: Add zscore

c7e0ca5

ENH: Add util/statistics to contrib/fom

38e3ab6

MAINT: Update licence of contrib/tensorflow

0acdc3c

DOC: Add readme to contrib/tensorflow

1737e0e

MAINT: imporovements to contrib/tensorflow

127814e

TST: Add rudimentary test for tensorflow

a97fcd5

MAINT: Minor improvements to tensorflow

e870da8

kohr-h approved these changes Aug 23, 2017

View reviewed changes

MAINT: Minor style fixes

9923fda

adler-j merged commit ea016dc into odlgroup:master Aug 29, 2017

adler-j deleted the tensorflow_support branch August 29, 2017 17:26

kohr-h removed the status: review needed label Oct 6, 2017

kohr-h added this to Done in Release 1.0.0 Sep 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for tensorflow together with ODL #972

Add support for tensorflow together with ODL #972

adler-j commented Apr 13, 2017

kohr-h commented Apr 13, 2017

adler-j commented Apr 16, 2017 •

edited

Loading

kohr-h commented May 18, 2017

adler-j commented May 19, 2017

kohr-h left a comment

kohr-h Jun 20, 2017

kohr-h Jun 20, 2017

kohr-h Jun 20, 2017

adler-j Jun 21, 2017

kohr-h Jun 20, 2017

kohr-h Jun 20, 2017

kohr-h Jun 20, 2017

adler-j Jun 21, 2017

kohr-h Jun 20, 2017

kohr-h Jun 20, 2017

kohr-h Jun 20, 2017

kohr-h Jun 20, 2017

kohr-h commented Jun 20, 2017

adler-j commented Jun 21, 2017

adler-j commented Aug 22, 2017

kohr-h left a comment

kohr-h Aug 23, 2017

kohr-h Aug 23, 2017

kohr-h Aug 23, 2017

adler-j Aug 24, 2017

kohr-h Aug 23, 2017

adler-j Aug 24, 2017

kohr-h Aug 23, 2017

adler-j Aug 24, 2017 •

edited

Loading

adler-j commented Aug 29, 2017

		@@ -0,0 +1,38 @@
		"""Example of how to convert an ODL operator to a tensorflow layer."""

		import tensorflow as tf


		"""Wrap ODL operator so that it acts on TensorflowSpace elements."""

		def __init__(self, domain, range, func, adjoint=None, linear=False):


		## Example usage

		The [examples](examples) folder contains example on how to use the above functionality.


		step = learning_rate * np.sqrt(1 - beta2) / (1 - beta1)

		x.lincomb(1, x, -step, m / (np.sqrt(v) + eps))

		@@ -802,6 +803,42 @@ def _apply_padding(lhs_arr, rhs_arr, offset, pad_mode, direction):
		working_slc[axis] = intersec_slc[axis]


		def zscore(arr):



		What is vectorization?
		======================

Add support for tensorflow together with ODL #972

Add support for tensorflow together with ODL #972

Conversation

adler-j commented Apr 13, 2017

kohr-h commented Apr 13, 2017

adler-j commented Apr 16, 2017 • edited Loading

kohr-h commented May 18, 2017

adler-j commented May 19, 2017

kohr-h 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

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

kohr-h commented Jun 20, 2017

adler-j commented Jun 21, 2017

adler-j commented Aug 22, 2017

kohr-h 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

Choose a reason for hiding this comment

adler-j Aug 24, 2017 • edited Loading

Choose a reason for hiding this comment

adler-j commented Aug 29, 2017

adler-j commented Apr 16, 2017 •

edited

Loading

adler-j Aug 24, 2017 •

edited

Loading