Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a c-api interface to initialize the thread environment of Paddle #5773

Merged
merged 7 commits into from
Dec 8, 2017

Conversation

Xreki
Copy link
Contributor

@Xreki Xreki commented Nov 20, 2017

if (isInit) return kPD_NO_ERROR;

if (FLAGS_use_gpu) {
hl_init(FLAGS_gpu_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hl_init will set the t_resource.device to -1, is it a bug?

@@ -43,4 +43,16 @@ paddle_error paddle_init(int argc, char** argv) {
isInit = true;
return kPD_NO_ERROR;
}

paddle_error paddle_init_thread() {
static __thread bool isInit = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not need 48 and 49 lines.
In the hl_init interface will determine whether it has been initialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

@@ -43,4 +43,16 @@ paddle_error paddle_init(int argc, char** argv) {
isInit = true;
return kPD_NO_ERROR;
}

paddle_error paddle_init_thread() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add an argument of device_id.
Users may want to initialize the thread to a different device environment.

Copy link
Contributor

@QingshuChen QingshuChen Nov 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If adding a device_id, another question is when user invokes the paddle_matrix_create API, which gpu is the matrix on? There is no device id in the paddle_matrix_create API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe need to wrap the hl_get_device interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to add device_id and set it to different values. The binary failed:

F1120 08:05:09.430253  1282 hl_cuda_device.cc:565] Check failed: cudaSuccess == cudaStat (0 vs. 77) Cuda Error: an illegal memory access was encountered

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This problem is due to the thread share the same parameter model with the main thread. In this case, you need to pass in the same device id as the main thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I know what your means. In the current mode, the Parameters are shared among multi-threads, but each thread has its own network. If each thread wants to run on different GPU, it should have its own network + Parameters. Then Parameters cannot be shared anymore, and we should not use the interface paddle_gradient_machine_create_shared_param but directly using paddle_gradient_machine_create_for_inference and paddle_gradient_machine_load_parameter_from_disk instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 我认为,这个多线程的例子,是希望使用paddle_gradient_machine_create_shared_param共享参数,不能支持跑在不同的GPU上。
  • 如果希望多个线程跑在不同的GPU上,则应该使用trainer_count>1或者,在每个线程里面单独地paddle_gradient_machine_create_for_inference以及paddle_gradient_machine_load_parameter_from_disk

@@ -27,4 +27,21 @@ typedef enum {
kPD_UNDEFINED_ERROR = -1,
} paddle_error;

static const char* paddle_error_string(paddle_error err) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the function implementation into a .cc file and avoid generating duplicate copies of code in the inference library.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

/**
* Initialize the thread environment of Paddle.
*/
PD_API paddle_error paddle_init_thread();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since some previous CPU users did not use this interface. Here need to indicate only GPU need this interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

hedaoyuan
hedaoyuan previously approved these changes Dec 8, 2017
#include <pthread.h>
#include <time.h>
#include "../common/common.h"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some comments about this example. For example, this is an inference implementation where multiple threads share a GPU.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

hedaoyuan
hedaoyuan previously approved these changes Dec 8, 2017
@Xreki Xreki merged commit 00b64f6 into PaddlePaddle:develop Dec 8, 2017
@Xreki Xreki deleted the fix_capi_multi_thread branch November 14, 2018 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants