Auto correct the location of model weights

[ghostplant]

The model update is completely equal to original version, while it doesn't require
user codes to reallocate the weights location among single_gpu and multi_gpu
environment, and the changed will make it more convenient to automatically work
under the best location (/cpu:0). Besides, users can also choose other location by
providing the value of weights_location argument.

Signed-off-by: CUI Wei ghostplant@qq.com

[fchollet]

There may be a significant performance cost to having a large subgraph on a single device (such as two copies of one large model). I believe this should be left to the user's discretion.

[ghostplant]

Thanks for you concerns. Here are the benchmark results about whether clone_model is applied:

InceptionV3 with multi_gpu_model:
GPU = NVIDIA Tesla P100 (4 GPUs, with 16GMEM each)

user-model-created-within-explicit-range-of-/cpu:0:
=> backend without clone_model: extra gpu_mem = 0, extra cpu_mem = 0.2GB/260GB, perf = 247ms/step
=> backend enabling clone_model: extra gpu_mem = 0, extra cpu_mem = 0.2GB/260GB, perf = 248ms/step

user-model-created-by-default (/gpu:0):
=> backend without clone_model: extra gpu_mem = 0.2GB/16GB, extra cpu_mem = 0, perf = 338ms/step
=> backend enabling clone_model: extra gpu_mem = 0.2GB/16GB, extra cpu_mem = 0, perf = 247ms/step

With the clone_model() patch added, the default model creation will only occupy <= 0.2GB gpu_mem for InceptionV3/ResNet50, since it is defined at gpu scope, while it didn't slow down the performance of training compared with the best way. This is proven by both InceptionV3 and ResNet50 fed with random inputs;

Since we see the performance of weights creation on GPU is extremely bad, we need quantities of code changes (e.g. additional with.device line, and wide-range of python indent trims) to make it work better for multiple GPUs, and after these modifications, the same source code is no longer suitable to run under single-GPU since the model turns to be defined on cpu-range thus it never uses single-GPU resource if the model cancels to call multi_gpu_model().

Weights relocation seems to be the most suitable way to make this sort of work convenient and get over massive code changes by users especially when the model defination jumps over many places in source code, while the method just brings about a little extra occupation of GPU memory only. So I think it would be necessary to support this method as an non-default option, which will be extremely helpful to users who don't quiet understand many related restrictions.

Besides, I support your concerns so legacy version is also very necessary to reserve as the default method.

[ghostplant]

I update the commit to make it an alternative option.

[fchollet]

Does anyone have any thoughts about this PR? cc @Dref360 @taehoonlee

[taehoonlee]

Thank you for the PR, @ghostplant. The example in the docs is:

with tf.device('/cpu:0'):
   model = ...
parallel_model = multi_gpu_model(model, gpus=8)

Do you assume that users create a base model on a GPU unlike the example?

[ghostplant]

@taehoonlee OK, the issue is that if users want to run mnist_cnn.py in the keras.examples, he/she has to change around 50% of the source code to support it, and when he/she decides to re-run the model using single GPU, still quantities of indent is needed to recover, otherwise, the code will be executed on CPU due to the force of with tf.device('/cpu:0').

[taehoonlee]

@fchollet @ghostplant, I think this is good idea. How about True/False valued cpu_relocation and cpu_merge, rather than the parameter weights_relocation with the argument /cpu:0? In other words:

if cpu_relocation:
    with tf.device('/cpu:0'):
        model = clone_model(model)
...
with tf.device('/cpu:0' if cpu_merge else '/gpu:%d' % target_gpu_ids[0]):
    merged = []
...

I think the following are possible cases:

• cpu_relocation=False, cpu_merge=True (the example in the docs): a base model exists on CPU and a user don't have NVLink.
• cpu_relocation=True, cpu_merge=True (the usecase assumed by @ghostplant): a base model exists on GPU and a user don't have NVLink.
• cpu_relocation=False, cpu_merge=False: all the models exist on GPUs with NVLink (gpu-gpu memcpy is faster than gpu-cpu memcpy through PCIe).

[ghostplant]

@taehoonlee I think shorten the number of arguments is better since it can be easier to understand by users.

Currently,
weights_location=None matches cpu_relocation=False, cpu_merge=True
weights_location='/cpu:0' matches cpu_relocation=True, cpu_merge=True
weights_location='/gpu:0' matches cpu_relocation=False, cpu_merge=False

Originally, the Keras API forces users to write with tf.device('/cpu:0'), so I think it is not hard to for them to understand what '/cpu:0' is. Besides, the single argument will also accept other location possibilities, such as weights_location='/gpu:1', weights_location='/localhost:3344/ps:0', so it can cover some more cases.

[taehoonlee]

@ghostplant, the last one weights_location='/gpu:0' does not match cpu_relocation=False, cpu_merge=False. That's why I propose another arguments.

In your codes, model = clone_model(model) will be performed when weights_location='/gpu:0'. This is unnecessary because a base model is already placed on GPU as you assumed.

[ghostplant]

@taehoonlee Yes. By the way, I have a multi-P100 GPUs with NVlink and I tested the keras.InceptionV3 case and see that define model on GPU:0 + merge weights on GPU:0 doesn't boost the performance, and it is still 40% slower than define model on CPU:0 + merge weights on CPU:0. Do you have the same problem using GPUs with NVlink?

[ghostplant]

@taehoonlee Maybe two arguments like model_relocation='/cpu:0' and merge_location='/cpu:0' will be more extensible. By default, model_relocation=None and merge_location='/cpu:0' to keep compatible with existing version.

[taehoonlee]

@ghostplant, I don't have NVLink. If your tests are correct, users never use the argument weights_relocation='/gpu:0' because that always show poor performance regardless of the location of a base model. Then, the parameter weights_relocation do not need to have extensibility, and it is enough to set the parameter binary.

Additionally, if the merge operation is efficient on GPUs, the gpu numbers should be assigned automatically, not by users. That's why there is the function _get_available_devices.

[ghostplant]

@taehoonlee I have new findings, merge_on_gpu will have 5%-10% boost using GPUs with NVlink but only when CUDA API cuCtxEnablePeerAccess() is invoked. If not, the performance will be worse than merge_on_cpu. So I think merge_on_gpu is still a necessary option.

[taehoonlee]

@ghostplant, Good finding. Then, do you have plans to revise this PR?

[ghostplant]

yes, i think using 2 arguments below will be the best: model_relocation='/cpu:0' and merge_location='/cpu:0', and your opinion?

[taehoonlee]

@ghostplant, I think it is better to have two binary arguments cpu_relocation and cpu_merge. First let's change as you proposed and then listen to François's feedback.

[ghostplant]

ok~

[fchollet]

Agreed that binary arguments (cpu_relocation and cpu_merge) would be more user-friendly than string arguments (model_relocation='/cpu:0').

[taehoonlee]

LGTM

[fchollet]

typo: "create the model's weights under..."

also prefer using "the CPU" since we always assume one CPU

[fchollet]

Typo: recommended

[ghostplant]

Updated~

[fchollet]

LGTM, thanks!