Improve TF backend's Switch function #7958

farizrahman4u · 2017-09-22T10:07:44Z

As of now, K.switch supports only scalar condition on TF backend. This PR makes TF backend's switch function behave more like Theano's switch; with element-wise selection and broadcasting when required.

If rank of condition == 0 (scalar), call tf.cond like before
If rank of condition == 1, call tf.where
If rank of condition > 1, make sure shape of condition is same as that of the expressions by applying expand_dims and tile ops; then call tf.where

farizrahman4u · 2017-09-22T11:42:08Z

@abhaikollara Tests :)

fchollet

Code looks good to me. Please add unit tests for the new cases.

fchollet · 2017-09-22T21:04:30Z

keras/backend/tensorflow_backend.py

+        if callable(else_expression):
+            else_expression = else_expression()
+        expr_ndim = ndim(then_expression)
+        assert cond_ndim <= expr_ndim, 'Rank of condition should be less'


Raise a ValueError instead of an assert (and add it to the docstring).

fchollet · 2017-09-22T21:05:07Z

keras/backend/tensorflow_backend.py

+            else_expression = else_expression()
+        expr_ndim = ndim(then_expression)
+        assert cond_ndim <= expr_ndim, 'Rank of condition should be less'
+        ' than or equal to rank of then and else expressions.'


The error message should mention the rank of the condition that was passed and the rank of the then/else expressions

farizrahman4u · 2017-09-24T17:29:04Z

Seems need to make changes to CNTK backend too.

fchollet · 2017-09-26T17:32:23Z

The CNTK issue seems straightforward: def tile(x, n): needs to be fixed to support integer n.

farizrahman4u · 2017-09-27T17:19:08Z

Seems there is another bug in cntk's tile.

farizrahman4u · 2017-09-28T07:26:10Z

Theano was broadcasting the wrong way: [(4,), (4, 3), (4, 3)] -> [(1, 4), (4, 3), (4, 3)] instead of [(4, 1), (4, 3), (4, 3)]

farizrahman4u · 2017-09-28T10:28:19Z

@fchollet Done (finally).

fchollet

LGTM, thanks. Note we are currently have an issue with Travis CI so I don't expect the tests to pass.

fchollet · 2017-09-28T18:15:01Z

keras/backend/cntk_backend.py

+    ndim_cond = ndim(condition)
+    ndim_expr = ndim(then_expression)
+    if ndim_cond > ndim_expr:
+            raise ValueError('Rank of condition should be less'


Reduce indent (should be 4 spaces)

fchollet · 2017-09-28T18:15:35Z

keras/backend/cntk_backend.py

+    ndim_expr = ndim(then_expression)
+    if ndim_cond > ndim_expr:
+            raise ValueError('Rank of condition should be less'
+                             ' than or equal to rank of then and'


Wrap then and else with ` to make the sentence easier to parse.

fchollet · 2017-09-28T18:16:07Z

keras/backend/tensorflow_backend.py

+        if cond_ndim > expr_ndim:
+            raise ValueError('Rank of condition should be less'
+                             ' than or equal to rank of then and'
+                             ' else expressions. ndim(condition)=' +


Wrap then and else with ` to make the sentence easier to parse.

~ is used for actual stuff in code right? So shouldn't it be then_expression and else_expression? Or simply then and else expressions?

Right, better to use then_expression etc.

fchollet

LGTM, thanks!

* improve k.switch * update doc * handle case when ndim is None * pep8 * use tf.reshape instead of tf.expand_dims * unit tests + better error msg * remove faulty cases * add broadcasting to cntk backend * typo * typo * update ds * update ds * cntk doesnt support symbolic tile * cntk doesnt support symbolic tile * allow int n for cntk tile * int->tuple * bug fix cntk tile * fix broadcasting in theano backend * formatting fixes

…-outputs * master: (68 commits) Change default value of shuffle parameter of Sequential.fit_generator() from True to False. (keras-team#8075) Fix off-by-one bug in predict/evaluate progress bar (keras-team#8071) Revert "Faster sequence" (keras-team#8060) Support NCHW for conv2d. (keras-team#8021) Change compute_accuracy() argument order and names (keras-team#8049) Replace literal constant 10 with variable num_classes in example/ (keras-team#8041) Faster sequence (keras-team#8039) Improve RNN docs. Enable accuracy reporting during training in examples/mnist_siamese_graph.py (keras-team#7997) Bug fix: Models with shared layers shouldn't be considered Sequential like (keras-team#8025) Add 'subtract' merge layer documentation (keras-team#8038) Update inference in seq2seq script to be more efficient Remove lstm_benchmark from examples/README.md (keras-team#8024) Add shuffle to the Model API (keras-team#8023) Add seq2seq example script. fix travis failure (keras-team#8014) Improve TF backend's Switch function (keras-team#7958) Added support for dynamic noise_shape in Dropout (keras-team#7999) Make on_epoch_end optional (keras-team#8007) Incremental tests speed ups. ...

farizrahman4u added 4 commits September 22, 2017 15:33

improve k.switch

4032480

update doc

0d408c1

handle case when ndim is None

c84359b

pep8

3247c00

use tf.reshape instead of tf.expand_dims

e0b7c5c

fchollet reviewed Sep 22, 2017

View reviewed changes

farizrahman4u added 2 commits September 24, 2017 00:14

unit tests + better error msg

cf889e5

remove faulty cases

73ae9a4

farizrahman4u added 7 commits September 25, 2017 00:02

add broadcasting to cntk backend

9fdb753

typo

210d71d

typo

2494edf

update ds

380f2a4

update ds

5d66408

cntk doesnt support symbolic tile

ffcc9ae

cntk doesnt support symbolic tile

58989d6

farizrahman4u closed this Sep 26, 2017

farizrahman4u reopened this Sep 26, 2017

farizrahman4u added 2 commits September 27, 2017 13:11

allow int n for cntk tile

2694987

int->tuple

f73ff05

farizrahman4u added 2 commits September 27, 2017 22:50

bug fix cntk tile

33a783a

fix broadcasting in theano backend

49ed074

fchollet previously approved these changes Sep 28, 2017

View reviewed changes

formatting fixes

ea594c3

farizrahman4u dismissed fchollet’s stale review via ea594c3 September 28, 2017 18:31

fchollet approved these changes Sep 29, 2017

View reviewed changes

fchollet merged commit 1bbfdb6 into keras-team:master Sep 29, 2017

farizrahman4u deleted the k_switch branch September 29, 2017 09:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve TF backend's Switch function #7958

Improve TF backend's Switch function #7958

farizrahman4u commented Sep 22, 2017 •

edited

Loading

farizrahman4u commented Sep 22, 2017

fchollet left a comment

fchollet Sep 22, 2017

fchollet Sep 22, 2017

farizrahman4u commented Sep 24, 2017

fchollet commented Sep 26, 2017

farizrahman4u commented Sep 27, 2017

farizrahman4u commented Sep 28, 2017

farizrahman4u commented Sep 28, 2017

fchollet left a comment

fchollet Sep 28, 2017

fchollet Sep 28, 2017

fchollet Sep 28, 2017

farizrahman4u Sep 28, 2017 •

edited

Loading

fchollet Sep 28, 2017

fchollet left a comment

Improve TF backend's Switch function #7958

Improve TF backend's Switch function #7958

Conversation

farizrahman4u commented Sep 22, 2017 • edited Loading

farizrahman4u commented Sep 22, 2017

fchollet left a comment

Choose a reason for hiding this comment

fchollet Sep 22, 2017

Choose a reason for hiding this comment

fchollet Sep 22, 2017

Choose a reason for hiding this comment

farizrahman4u commented Sep 24, 2017

fchollet commented Sep 26, 2017

farizrahman4u commented Sep 27, 2017

farizrahman4u commented Sep 28, 2017

farizrahman4u commented Sep 28, 2017

fchollet left a comment

Choose a reason for hiding this comment

fchollet Sep 28, 2017

Choose a reason for hiding this comment

fchollet Sep 28, 2017

Choose a reason for hiding this comment

fchollet Sep 28, 2017

Choose a reason for hiding this comment

farizrahman4u Sep 28, 2017 • edited Loading

Choose a reason for hiding this comment

fchollet Sep 28, 2017

Choose a reason for hiding this comment

fchollet left a comment

Choose a reason for hiding this comment

farizrahman4u commented Sep 22, 2017 •

edited

Loading

farizrahman4u Sep 28, 2017 •

edited

Loading