cppflow icon indicating copy to clipboard operation
cppflow copied to clipboard

Avoid default global context initialization before session options are provided.

Open ljn917 opened this issue 4 years ago • 8 comments

Try to ensure only a single context is created per process.

For example, in multi-GPU environment, the default context tries to use all the GPU. Limiting GPU usage by remapping GPU using session options is not possible.

Fix #126

ljn917 avatar May 31 '21 16:05 ljn917

@seungtaek94 Can you try again? Sorry for the wrong version.

On Mon, May 31, 2021 at 8:05 PM Seungtaek Kim @.***> wrote:

@.**** commented on this pull request.

In docs/source/quickstart.rst https://github.com/serizba/cppflow/pull/127#discussion_r642710368:

@@ -140,8 +140,8 @@ You can specify TensorFlow's GPU Options to prevent it from reserving all your G // Create new options with your configuration TFE_ContextOptions* options = TFE_NewContextOptions(); TFE_ContextOptionsSetConfig(options, config.data(), config.size(), cppflow::context::get_status());

  • // Replace the global context with your options
  • cppflow::get_global_context() = cppflow::context(options);
  • // Initialize the global context with your options
  • cppflow::context::set_context_options(opts);

If i call this function like this. It throws the error Call to non-static-member function without an object argument

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/serizba/cppflow/pull/127#pullrequestreview-672572561, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALDTHTWBUC6NDGGPSKY55TTQQP6LANCNFSM453GAWLA .

ljn917 avatar Jun 01 '21 01:06 ljn917

@seungtaek94 Can you also test this branch for me? The differences are in how you create a model. For cppflow2-fix-context2 branch, you don't need to pass in the config vector to the model constructor.

@serizba I am not sure if you like the behavior of https://github.com/ljn917/cppflow/tree/cppflow2-fix-context2 branch. It relies on the memory layout of TFE_ContextOptions. The TFE_ContextOptions defines TF_SessionOptions at the beginning of the struct, so we can cast the pointer type. Introducing the definition of TFE_ContextOptions seems to be impossible, because TF_SessionOptions is an opaque C++ type.

BTW, please don't merge at this time.

ljn917 avatar Jun 01 '21 02:06 ljn917

I tested ljn917:cppflow2-fix-context branch with this code.

And it works.

But can we handle to use different context with different object like below? Now it seems like cant handle.

std::vector < uint8_t> gpu0 = { 0x32,0xc,0x9,0x9a,0x99,0x99,0x99,0x99,0x99,0xa9,0x3f,0x2a,0x1,0x30,0x38,0x1 };
std::vector < uint8_t> gpu1 = { 0x32,0xc,0x9,0x9a,0x99,0x99,0x99,0x99,0x99,0xa9,0x3f,0x2a,0x1,0x31,0x38,0x1 };

std::string strModelPath = "C:/Users/admin/source/repos/TestToLoadFrozenGraph/src/models/test";
std::string strImgPath = "C:/Users/admin/source/repos/TestToLoadFrozenGraph/src/my_cat.jpg";
	
Inference* inferWithGpu0 = new Inference(strModelPath, gpu0);
inferWithGpu0 ->run(strImgPath);
delete inferWithGpu0 ;

Inference* inferWithGpu1= new Inference(strModelPath, gpu1);
inferWithGpu1->run(strImgPath);
delete inferWithGpu1;

@seungtaek94 Can you try again? Sorry for the wrong version. On Mon, May 31, 2021 at 8:05 PM Seungtaek Kim @.> wrote: @.* commented on this pull request. ------------------------------ In docs/source/quickstart.rst <#127 (comment)>: > @@ -140,8 +140,8 @@ You can specify TensorFlow's GPU Options to prevent it from reserving all your G // Create new options with your configuration TFE_ContextOptions* options = TFE_NewContextOptions(); TFE_ContextOptionsSetConfig(options, config.data(), config.size(), cppflow::context::get_status()); - // Replace the global context with your options - cppflow::get_global_context() = cppflow::context(options); + // Initialize the global context with your options + cppflow::context::set_context_options(opts); If i call this function like this. It throws the error Call to non-static-member function without an object argument — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#127 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALDTHTWBUC6NDGGPSKY55TTQQP6LANCNFSM453GAWLA .

seungtaek94 avatar Jun 01 '21 03:06 seungtaek94

ljn917:cppflow2-fix-context2 this branch also works.

I tested both branch with win10, rtx 2080 super * 2.

@seungtaek94 Can you also test this branch for me? The differences are in how you create a model. For cppflow2-fix-context2 branch, you don't need to pass in the config vector to the model constructor.

@serizba I am not sure if you like the behavior of https://github.com/ljn917/cppflow/tree/cppflow2-fix-context2 branch. It relies on the memory layout of TFE_ContextOptions. The TFE_ContextOptions defines TF_SessionOptions at the beginning of the struct, so we can cast the pointer type. Introducing the definition of TFE_ContextOptions seems to be impossible, because TF_SessionOptions is an opaque C++ type.

BTW, please don't merge at this time.

seungtaek94 avatar Jun 01 '21 03:06 seungtaek94

@seungtaek94 Having two contexts for two different GPUs separately is not possible in tensorflow. Please see https://github.com/tensorflow/tensorflow/issues/18861 and https://github.com/tensorflow/tensorflow/issues/19083 for details. The workaround is to have a global context containing all the GPUs and set the GPU device for each graph. Here is an example with the frozen graph. This is another example that can be used to modify TF_LoadSessionFromSavedModel(). This is how another framework handles it.

ljn917 avatar Jun 01 '21 04:06 ljn917

@ljn917 Thanks for information.

So for my case, I need to set global context with all GPU(two gpus) and then set device for each graph. Am i correct?

seungtaek94 avatar Jun 01 '21 04:06 seungtaek94

Yes, that is correct.

On Tue, Jun 1, 2021, 00:38 Seungtaek Kim @.***> wrote:

@ljn917 https://github.com/ljn917 Thanks for information.

So for my case, I need to set global context with all GPU(two gpus) and then set device for each graph. Am i correct?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/serizba/cppflow/pull/127#issuecomment-851798097, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALDTHROFEPP7CYJ5ZVSJATTQRP4TANCNFSM453GAWLA .

ljn917 avatar Jun 01 '21 04:06 ljn917

Thanks. Anyway, both branch those you mentioned, it works :)

seungtaek94 avatar Jun 01 '21 05:06 seungtaek94

I didn't test this cause I don't have a multi GPU setting. Should I still fix these conflicts and merge this?

serizba avatar Sep 23 '22 16:09 serizba

@serizba I guess no.

First, there is a problem in the construction of the global context, i.e. it uses all GPUs and cannot be changed. (as described in #126 ) Most people can get around this by setting CUDA_VISIBLE_DEVICES.

The real issue here is that we have no means of running two graphs (or models) on two GPUs separately (graph0 on GPU0, and graph1 on GPU1). The default setting simply runs graph0 on GPU0 and GPU1, and it is the same for graph1. The solution I found was in a previous comment https://github.com/serizba/cppflow/pull/127#issuecomment-851793585. Basically, we need to rewrite the graphs by adding GPU ID information, which cannot be done easily with the TF C API.

ljn917 avatar Sep 28 '22 04:09 ljn917

Mmmmm... I see.

Then I proceed to close this. Thanks!

serizba avatar Sep 29 '22 08:09 serizba