Skip to content

Commit 559ff28

Browse files
committed
Prevent None from becoming a control dependency
Summary: Depending on the optimizer wrapped, None can be returned. Fix T43585 TF2.4 Only Test Plan: CI, added a test which failed before Reviewers: jackh, alfiee, samuelh, #tensorflow, simonl, #framework_ip_review_-_any_oss_or_third-party_code_use_has_been_approved, jakeh Reviewed By: #tensorflow, #framework_ip_review_-_any_oss_or_third-party_code_use_has_been_approved, jakeh Maniphest Tasks: T43585 Differential Revision: https://phabricator.sourcevertex.net/D49237
1 parent 0e6c80d commit 559ff28

File tree

4 files changed

+18
-12
lines changed

4 files changed

+18
-12
lines changed

tensorflow/python/ipu/keras/optimizers/gradient_accumulate_optimizer.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ def _resource_apply_dense(self, grad, handle, apply_state):
120120
def resource_update_():
121121
updated_var = self._opt._resource_apply_dense( # pylint: disable=protected-access
122122
acc_grad, acc_var, apply_state)
123-
apply_grad_ops.append(updated_var)
123+
if updated_var is not None:
124+
apply_grad_ops.append(updated_var)
124125

125126
return GradientAccumulationOptimizerV2.apply_gradient_accumulation(
126127
resource_update_,

tensorflow/python/ipu/ops/pipelining_ops.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,15 +1009,15 @@ def resource_update_():
10091009
apply_grads = opt.apply_gradients(accumulated_grads_and_vars,
10101010
*apply_gradients_args,
10111011
**apply_gradients_kwargs)
1012-
if apply_grads:
1012+
if apply_grads is not None:
10131013
resource_update_ops.append(apply_grads)
10141014

10151015
# Enqueue any accumulated outfeed data
10161016
if outfeed_sinks:
10171017
# Note: unpack if we're outfeeding loss.
10181018
to_enqueue = outfeed_sinks[0] if outfeed_loss else outfeed_sinks
10191019
enqueue = outfeed_queue.enqueue(to_enqueue)
1020-
if enqueue:
1020+
if enqueue is not None:
10211021
resource_update_ops.append(enqueue)
10221022

10231023
with ops.name_scope(name + "/WU") as scope:

tensorflow/python/ipu/optimizers/gradient_accumulation_optimizer.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,8 @@ def apply_gradients(self, grads_and_vars, global_step=None, name=None):
187187
def resource_update_():
188188
apply_grads = self._opt.apply_gradients(accumulated_grads_and_vars,
189189
global_step, name)
190-
apply_grad_ops.append(apply_grads)
190+
if apply_grads is not None:
191+
apply_grad_ops.append(apply_grads)
191192

192193
return self.apply_gradient_accumulation(
193194
resource_update_, self._opt.get_name(), apply_grad_ops,

tensorflow/python/ipu/tests/keras/keras_gradient_accumulation_test.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from tensorflow.python.data.ops import dataset_ops
2323
from tensorflow.python.framework import test_util
2424
from tensorflow.python.platform import test
25+
from tensorflow.python.training import gradient_descent
2526

2627

2728
def get_mnist_dataset(batch_size):
@@ -63,24 +64,28 @@ class KerasGradientAccumulationTest(test.TestCase, parameterized.TestCase):
6364
TESTCASES = [{
6465
"testcase_name": "sequential",
6566
"model_fn": simple_sequential_model,
66-
"replication_factor": 1
67+
"replication_factor": 1,
68+
"optimizer": "sgd"
6769
}, {
6870
"testcase_name": "sequential_replicated",
6971
"model_fn": simple_sequential_model,
70-
"replication_factor": 2
72+
"replication_factor": 2,
73+
"optimizer": "sgd"
7174
}, {
7275
"testcase_name": "functional",
7376
"model_fn": simple_functional_model,
74-
"replication_factor": 1
77+
"replication_factor": 1,
78+
"optimizer": "sgd"
7579
}, {
7680
"testcase_name": "functional_replicated",
7781
"model_fn": simple_functional_model,
78-
"replication_factor": 2
82+
"replication_factor": 2,
83+
"optimizer": gradient_descent.GradientDescentOptimizer(0.001)
7984
}]
8085

8186
@parameterized.named_parameters(*TESTCASES)
8287
@test_util.run_v2_only
83-
def testModels(self, model_fn, replication_factor):
88+
def testModels(self, model_fn, replication_factor, optimizer):
8489
tu.skip_if_not_enough_ipus(self, replication_factor)
8590

8691
cfg = ipu.config.IPUConfig()
@@ -98,8 +103,7 @@ def testModels(self, model_fn, replication_factor):
98103
# Run on CPU - simulate gradient accumulation by just using a bigger batch
99104
# size but less steps per epoch.
100105
m = model_fn()
101-
m.compile(optimizer='sgd',
102-
loss=keras.losses.SparseCategoricalCrossentropy())
106+
m.compile(optimizer, loss=keras.losses.SparseCategoricalCrossentropy())
103107
m.fit(get_mnist_dataset(batch_size * gradient_accumulation_steps),
104108
steps_per_epoch=steps_per_epoch // gradient_accumulation_steps,
105109
epochs=epochs)
@@ -108,7 +112,7 @@ def testModels(self, model_fn, replication_factor):
108112
strategy = ipu.ipu_strategy.IPUStrategyV1()
109113
with strategy.scope():
110114
m = model_fn()
111-
m.compile(optimizer='sgd',
115+
m.compile(optimizer,
112116
loss=keras.losses.SparseCategoricalCrossentropy(),
113117
steps_per_execution=gradient_accumulation_steps * 2)
114118
m.set_gradient_accumulation_options(

0 commit comments

Comments
 (0)