From 47b78e8d6babb59ae44d25fa8e810609116ce997 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Thu, 28 Feb 2019 16:19:23 +0000 Subject: [PATCH 1/2] Simplify python loops This PR replaces loops over range(len(...)) with enumerate() or zip(). This makes the code (arguably) easier to read and in some cases even slightly faster. --- tensorflow/python/debug/cli/analyzer_cli.py | 3 +-- .../python/debug/cli/debugger_cli_common.py | 8 ++------ .../python/debug/cli/profile_analyzer_cli.py | 4 ++-- tensorflow/python/debug/cli/tensor_format.py | 4 ++-- tensorflow/python/debug/lib/debug_graphs.py | 3 +-- .../python/distribute/cross_device_ops.py | 4 ++-- tensorflow/python/eager/backprop.py | 6 +++--- .../keras/engine/training_distributed.py | 3 +-- .../python/keras/engine/training_utils.py | 9 ++++----- tensorflow/python/keras/models.py | 3 +-- .../python/keras/utils/multi_gpu_utils.py | 8 +++----- tensorflow/python/ops/data_flow_ops.py | 5 ++--- tensorflow/python/ops/gradients_util.py | 20 +++++++++---------- tensorflow/python/ops/variable_scope.py | 6 ++---- .../python/training/checkpoint_management.py | 6 ++---- 15 files changed, 37 insertions(+), 55 deletions(-) diff --git a/tensorflow/python/debug/cli/analyzer_cli.py b/tensorflow/python/debug/cli/analyzer_cli.py index 224fb451ab6..5c21b1518ed 100644 --- a/tensorflow/python/debug/cli/analyzer_cli.py +++ b/tensorflow/python/debug/cli/analyzer_cli.py @@ -1438,8 +1438,7 @@ class DebugAnalyzer(object): hang += DEPTH_TEMPLATE % depth - for i in xrange(len(all_inputs)): - inp = all_inputs[i] + for i, inp in enumerate(all_inputs): op_type = self._debug_dump.node_op_type(debug_graphs.get_node_name(inp)) if op_type in self._GRAPH_STRUCT_OP_TYPE_BLACKLIST: continue diff --git a/tensorflow/python/debug/cli/debugger_cli_common.py b/tensorflow/python/debug/cli/debugger_cli_common.py index 02563fde845..ed02d6a479d 100644 --- a/tensorflow/python/debug/cli/debugger_cli_common.py +++ b/tensorflow/python/debug/cli/debugger_cli_common.py @@ -25,7 +25,6 @@ import traceback import numpy as np import six -from six.moves import xrange # pylint: disable=redefined-builtin from tensorflow.python import pywrap_tensorflow_internal from tensorflow.python.platform import gfile @@ -412,8 +411,7 @@ def regex_find(orig_screen_output, regex, font_attr): raise ValueError("Invalid regular expression: \"%s\"" % regex) regex_match_lines = [] - for i in xrange(len(new_screen_output.lines)): - line = new_screen_output.lines[i] + for i, line in enumerate(new_screen_output.lines): find_it = re_prog.finditer(line) match_segs = [] @@ -466,11 +464,9 @@ def wrap_rich_text_lines(inp, cols): out = RichTextLines([]) row_counter = 0 # Counter for new row index - for i in xrange(len(inp.lines)): + for i, line in enumerate(inp.lines): new_line_indices.append(out.num_lines()) - line = inp.lines[i] - if i in inp.annotations: out.annotations[row_counter] = inp.annotations[i] diff --git a/tensorflow/python/debug/cli/profile_analyzer_cli.py b/tensorflow/python/debug/cli/profile_analyzer_cli.py index a384d255ba5..0e846d2d2d9 100644 --- a/tensorflow/python/debug/cli/profile_analyzer_cli.py +++ b/tensorflow/python/debug/cli/profile_analyzer_cli.py @@ -568,8 +568,8 @@ class ProfileAnalyzer(object): # Add stat totals. row_str = "" - for col in range(len(device_total_row)): - row_str += ("{:<%d}" % column_widths[col]).format(device_total_row[col]) + for width, row in zip(column_widths, device_total_row): + row_str += ("{:<%d}" % width).format(row) output.append(RL()) output.append(RL(row_str)) return debugger_cli_common.rich_text_lines_from_rich_line_list(output) diff --git a/tensorflow/python/debug/cli/tensor_format.py b/tensorflow/python/debug/cli/tensor_format.py index 9ba84e3f226..374fcd58bc3 100644 --- a/tensorflow/python/debug/cli/tensor_format.py +++ b/tensorflow/python/debug/cli/tensor_format.py @@ -249,8 +249,8 @@ def _annotate_ndarray_lines( curr_indices = [0] * len(dims) curr_dim = 0 - for i in xrange(len(array_lines)): - line = array_lines[i].strip() + for i, raw_line in enumerate(array_lines): + line = raw_line.strip() if not line: # Skip empty lines, which can appear for >= 3D arrays. diff --git a/tensorflow/python/debug/lib/debug_graphs.py b/tensorflow/python/debug/lib/debug_graphs.py index 8c95cd2e17e..0de9e22d88d 100644 --- a/tensorflow/python/debug/lib/debug_graphs.py +++ b/tensorflow/python/debug/lib/debug_graphs.py @@ -346,8 +346,7 @@ class DebugGraph(object): for node in self._node_inputs: inputs = self._node_inputs[node] - for i in xrange(len(inputs)): - inp = inputs[i] + for i, inp in enumerate(inputs): if is_copy_node(inp): # Find the input to the Copy node, which should be the original # input to the node. diff --git a/tensorflow/python/distribute/cross_device_ops.py b/tensorflow/python/distribute/cross_device_ops.py index 67578ba0c6f..a15a3309e02 100644 --- a/tensorflow/python/distribute/cross_device_ops.py +++ b/tensorflow/python/distribute/cross_device_ops.py @@ -933,8 +933,8 @@ class MultiWorkerAllReduce(AllReduceCrossDeviceOps): aggregated_grads = range_agg_grads else: assert len(aggregated_grads) == len(range_agg_grads) - for i in range(len(aggregated_grads)): - aggregated_grads[i] += range_agg_grads[i] + for i, range_agg_grad in enumerate(range_agg_grads): + aggregated_grads[i] += range_agg_grad assert not remaining_grads return _ungroup_and_make_mirrored(aggregated_grads, per_replica_values[0], diff --git a/tensorflow/python/eager/backprop.py b/tensorflow/python/eager/backprop.py index e0b8b341da2..e5d4b2523df 100644 --- a/tensorflow/python/eager/backprop.py +++ b/tensorflow/python/eager/backprop.py @@ -514,9 +514,9 @@ def make_vjp(f, params=None, persistent=True): try: sources = [] args = [ - ops.convert_to_tensor(args[i]) - if i in parameter_positions else args[i] - for i in range(len(args)) + ops.convert_to_tensor(arg) + if i in parameter_positions else arg + for i, arg in enumerate(args) ] args = _ensure_unique_tensor_objects(parameter_positions, args) for i in parameter_positions: diff --git a/tensorflow/python/keras/engine/training_distributed.py b/tensorflow/python/keras/engine/training_distributed.py index 4c309284ab0..71554bd6bad 100644 --- a/tensorflow/python/keras/engine/training_distributed.py +++ b/tensorflow/python/keras/engine/training_distributed.py @@ -565,8 +565,7 @@ def experimental_tpu_predict_loop(model, prediction_result = np.concatenate(unconcatenated_outs[0], axis=0) else: prediction_result = [ - np.concatenate(unconcatenated_outs[i], axis=0) - for i in range(len(unconcatenated_outs)) + np.concatenate(out, axis=0) for out in unconcatenated_outs ] if padding_handler: diff --git a/tensorflow/python/keras/engine/training_utils.py b/tensorflow/python/keras/engine/training_utils.py index 64be9eafeb1..ace5bca641b 100644 --- a/tensorflow/python/keras/engine/training_utils.py +++ b/tensorflow/python/keras/engine/training_utils.py @@ -27,6 +27,7 @@ import time import numpy as np import six +from six.moves import zip # pylint: disable=redefined-builtin from tensorflow.python import tf2 from tensorflow.python.data.experimental.ops import cardinality @@ -1589,9 +1590,7 @@ class ModelInputs(object): # TODO(karmel): There is a side-effect here where what you get # with as_list and as_dict depends on whether you have called this # method first, since it modifies in place. - for i in range(len(self._flattened_inputs)): - k = self._input_names[i] - v = self._flattened_inputs[i] + for i, (k, v) in enumerate(zip(self._input_names, self._flattened_inputs)): if isinstance(v, (list, float, int)): v = np.asarray(v) if v.ndim == 1: @@ -1621,8 +1620,8 @@ class ModelInputs(object): def as_dict(self): """An iterable over a dictionary version of inputs.""" - for i in range(len(self._flattened_inputs)): - yield self._input_names[i], self._flattened_inputs[i] + for k, v in zip(self._input_names, self._flattened_inputs): + yield k, v def as_list(self): """Returning the inputs as a list.""" diff --git a/tensorflow/python/keras/models.py b/tensorflow/python/keras/models.py index d699daf6b48..657938fc885 100644 --- a/tensorflow/python/keras/models.py +++ b/tensorflow/python/keras/models.py @@ -116,8 +116,7 @@ def _clone_functional_model(model, input_tensors=None, layer_fn=_clone_layer): # If tensor comes from an input layer: cache the input layer. input_tensors = nest.flatten(input_tensors) input_tensors_ = [] - for i in range(len(input_tensors)): - input_tensor = input_tensors[i] + for i, input_tensor in enumerate(input_tensors): if not K.is_keras_tensor(input_tensor): original_input_layer = model._input_layers[i] name = original_input_layer.name diff --git a/tensorflow/python/keras/utils/multi_gpu_utils.py b/tensorflow/python/keras/utils/multi_gpu_utils.py index 9eeade4cf08..757098c2d8d 100644 --- a/tensorflow/python/keras/utils/multi_gpu_utils.py +++ b/tensorflow/python/keras/utils/multi_gpu_utils.py @@ -211,9 +211,7 @@ def multi_gpu_model(model, gpus, cpu_merge=True, cpu_relocation=False): with ops.device('/cpu:0'): model = clone_model(model) - all_outputs = [] - for i in range(len(model.outputs)): - all_outputs.append([]) + all_outputs = [[] for _ in range(len(model.outputs))] # Place a copy of the model on each GPU, # each getting a slice of the inputs. @@ -241,8 +239,8 @@ def multi_gpu_model(model, gpus, cpu_merge=True, cpu_relocation=False): outputs = [outputs] # Save the outputs for merging back together later. - for o in range(len(outputs)): - all_outputs[o].append(outputs[o]) + for o, output in enumerate(outputs): + all_outputs[o].append(output) # Deduplicate output names to handle Siamese networks. occurrences = {} diff --git a/tensorflow/python/ops/data_flow_ops.py b/tensorflow/python/ops/data_flow_ops.py index 648453f3569..bf7314d918d 100644 --- a/tensorflow/python/ops/data_flow_ops.py +++ b/tensorflow/python/ops/data_flow_ops.py @@ -2426,8 +2426,7 @@ class RecordInput(object): with ops.name_scope(self._name): batch_list = [[] for _ in six.moves.range(self._batches)] records = array_ops.split(records, self._batch_size, 0) - records = [array_ops.reshape(record, []) for record in records] - for index, protobuf in zip(six.moves.range(len(records)), records): + for index, protobuf in enumerate(records): batch_index = index % self._batches - batch_list[batch_index].append(protobuf) + batch_list[batch_index].append(array_ops.reshape(protobuf, [])) return batch_list diff --git a/tensorflow/python/ops/gradients_util.py b/tensorflow/python/ops/gradients_util.py index eb9965cbf38..84a21d0bab5 100644 --- a/tensorflow/python/ops/gradients_util.py +++ b/tensorflow/python/ops/gradients_util.py @@ -21,7 +21,7 @@ from __future__ import print_function import collections import contextlib -from six.moves import xrange # pylint: disable=redefined-builtin +from six.moves import xrange, zip # pylint: disable=redefined-builtin from tensorflow.core.framework import attr_value_pb2 from tensorflow.python.eager import backprop @@ -160,9 +160,7 @@ def _DefaultGradYs(grad_ys, raise ValueError("Passed %d grad_ys for %d ys" % (len(grad_ys), len(ys))) grad_ys = ops.convert_n_to_tensor_or_indexed_slices(grad_ys, name="grad_y") new_grad_ys = [] - for i in xrange(len(grad_ys)): - grad_y = grad_ys[i] - y = ys[i] + for i, (y, grad_y) in enumerate(zip(ys, grad_ys)): with _maybe_colocate_with(y.op, gradient_uid, colocate_gradients_with_ops): if grad_y is None: if y.dtype.is_complex: @@ -883,23 +881,23 @@ class AggregationMethod(object): aggregating gradients: * `ADD_N`: All of the gradient terms are summed as part of one - operation using the "AddN" op (see `tf.add_n`). This - method has the property that all gradients must be ready and + operation using the "AddN" op (see `tf.add_n`). This + method has the property that all gradients must be ready and buffered separately in memory before any aggregation is performed. * `DEFAULT`: The system-chosen default aggregation method. - The following aggregation methods are experimental and may not + The following aggregation methods are experimental and may not be supported in future releases: * `EXPERIMENTAL_TREE`: Gradient terms are summed in pairs using - using the "AddN" op. This method of summing gradients may reduce - performance, but it can improve memory utilization because the + using the "AddN" op. This method of summing gradients may reduce + performance, but it can improve memory utilization because the gradients can be released earlier. * `EXPERIMENTAL_ACCUMULATE_N`: Gradient terms are summed using the - "AccumulateN" op (see `tf.accumulate_n`), which accumulates the + "AccumulateN" op (see `tf.accumulate_n`), which accumulates the overall sum in a single buffer that is shared across threads. - This method of summing gradients can result in a lower memory footprint + This method of summing gradients can result in a lower memory footprint and lower latency at the expense of higher CPU/GPU utilization. For gradients of types that "AccumulateN" does not support, this summation method falls back on the behavior of `EXPERIMENTAL_TREE` diff --git a/tensorflow/python/ops/variable_scope.py b/tensorflow/python/ops/variable_scope.py index a5502f01275..e2af4db803f 100644 --- a/tensorflow/python/ops/variable_scope.py +++ b/tensorflow/python/ops/variable_scope.py @@ -28,7 +28,7 @@ import traceback import six from six import iteritems -from six.moves import xrange # pylint: disable=redefined-builtin +from six.moves import xrange, zip # pylint: disable=redefined-builtin from tensorflow.python import tf2 from tensorflow.python.eager import context @@ -93,9 +93,7 @@ class _PartitionInfo(object): "full_shape is of length {}.".format( len(var_offset), len(full_shape))) - for i in xrange(len(full_shape)): - offset = var_offset[i] - shape = full_shape[i] + for offset, shape in zip(var_offset, full_shape): if offset < 0 or offset >= shape: raise ValueError( "Expected 0 <= offset < shape but found offset={}, shape={} for " diff --git a/tensorflow/python/training/checkpoint_management.py b/tensorflow/python/training/checkpoint_management.py index b4ad26275ab..32b9c023aae 100644 --- a/tensorflow/python/training/checkpoint_management.py +++ b/tensorflow/python/training/checkpoint_management.py @@ -108,8 +108,7 @@ def generate_checkpoint_state_proto(save_dir, if not os.path.isabs(save_dir): if not os.path.isabs(model_checkpoint_path): model_checkpoint_path = os.path.relpath(model_checkpoint_path, save_dir) - for i in range(len(all_model_checkpoint_paths)): - p = all_model_checkpoint_paths[i] + for i, p in enumerate(all_model_checkpoint_paths): if not os.path.isabs(p): all_model_checkpoint_paths[i] = os.path.relpath(p, save_dir) @@ -281,8 +280,7 @@ def get_checkpoint_state(checkpoint_dir, latest_filename=None): if not os.path.isabs(ckpt.model_checkpoint_path): ckpt.model_checkpoint_path = os.path.join(checkpoint_dir, ckpt.model_checkpoint_path) - for i in range(len(ckpt.all_model_checkpoint_paths)): - p = ckpt.all_model_checkpoint_paths[i] + for i, p in enumerate(ckpt.all_model_checkpoint_paths): if not os.path.isabs(p): ckpt.all_model_checkpoint_paths[i] = os.path.join(checkpoint_dir, p) except errors.OpError as e: From b7555d118e51c1a786509c939aeac6f87a91a6b1 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Thu, 28 Feb 2019 17:11:41 +0000 Subject: [PATCH 2/2] Remove unused import --- tensorflow/python/debug/lib/debug_graphs.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tensorflow/python/debug/lib/debug_graphs.py b/tensorflow/python/debug/lib/debug_graphs.py index 0de9e22d88d..b2d552c88e1 100644 --- a/tensorflow/python/debug/lib/debug_graphs.py +++ b/tensorflow/python/debug/lib/debug_graphs.py @@ -17,8 +17,6 @@ from __future__ import absolute_import from __future__ import division from __future__ import print_function -from six.moves import xrange # pylint: disable=redefined-builtin - from tensorflow.core.framework import graph_pb2 from tensorflow.python.framework import op_def_registry from tensorflow.python.platform import tf_logging as logging