Potential bug in resnet_run_loop dataset code

[ed-alertedh]

System information

• What is the top-level directory of the model you are using: models/official/resnet
• Have I written custom code (as opposed to using a stock example script provided in TensorFlow): No
• OS Platform and Distribution (e.g., Linux Ubuntu 16.04):N/A
• TensorFlow installed from (source or binary): N/A
• TensorFlow version (use command below): N/A
• Bazel version (if compiling from source): N/A
• CUDA/cuDNN version: N/A
• GPU model and memory: N/A
• Exact command to reproduce: N/A

Describe the problem

I was looking at the resnet code as an example of a high performance input pipeline and noticed that the final prefetch op on the dataset is not assigned to anything:

models/official/resnet/resnet_run_loop.py

Line 91 in fd1d178

dataset.prefetch(buffer_size=tf.contrib.data.AUTOTUNE)

Unless prefetch mutates the dataset (the docs don't indicate that it does), that line doesn't appear to be having its intended effect.

[ed-alertedh]

Looks like the code has had some churn recently. The line in question was re-introduced in 18d05ad

[robieta]

Hm. You may well be correct. I'm not seeing any effect in from a spot check. The instance I had handy was a 4x NVIDIA P100 box, but it may only make a difference on beefier hardware. I'll try to test it more tomorrow.

Adding @guptapriya who is also involved in ResNet pipelining.

[guptapriya]

Yes, that's definitely a problem! Thanks for the catch @ed-alertedh !
@robieta yeah it will hopefully make a difference on V100s. But we should fix it regardless because it's a bug as it is.

[ed-alertedh]

No problem! Just to test how well I'm understanding all this - the reason it doesn't matter on P100s is because MirroredStrategy has inserted a pipeline/prefetch_to_device stage which is hiding any latency on the CPU side?

[robieta]

@ed-alertedh I think it doesn't make a difference on P100s because the input pipeline is adequately fast already.

@guptapriya I'm seeing ~100 images/sec improvement (~2%) on both 8xV100 GCE and DGX-1. So no question that this should go in.

@tfboyd @reedwm Just FYI.

[guptapriya]

@robieta thanks for verifying!
@ed-alertedh yeah you're right that we also have the additional prefetch to device that could be indirectly causing prefetching on the CPU as well - but this would apply to both P100 and V100. the difference between those just comes from the fact that when the GPU gets faster, input pipeline needs to do even better to keep up, like Taylor said.