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

fix EXTERNAL_LIBS in CMakeLists.txt #3561

Merged
merged 1 commit into from
Aug 18, 2017
Merged

Conversation

yu239-zz
Copy link

@yu239-zz yu239-zz commented Aug 17, 2017

The variable 'EXTERNAL_LIB' in the original CMakeLists.txt seems not be used by any other cmake file. I believe that it should be 'EXTERNAL_LIBS'. I did have a CUDA computation issue with the current version of CMakeLists.txt. After changing it to 'EXTERNAL_LIBS', the issue seems to be fixed.

@skylian
Copy link

skylian commented Aug 17, 2017

Let me elaborate the "CUDA computation issue". If we don't change EXTERNAL_LIB to EXTERNAL_LIBS, we encountered two problems when running multiple threads that share the same CUDA stream:

  1. the numerical results are not correct
  2. With higher concurrency, after some time, the threads will hang at cudaStreamSynchronize's call clock_gettime(), and GPU utility will be 100% all the time.

One question is, if we don't correctly provide the CUDA runtime libraries to Paddle in cmake configuration, why can we still successfully build and run Paddle?

@skylian
Copy link

skylian commented Aug 18, 2017

I'm sorry. Please ignore my comment above. We cannot replicate the problem by changing back to EXTERNAL_LIB.

Copy link
Contributor

@gangliao gangliao left a comment

Choose a reason for hiding this comment

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

LGTM

@gangliao gangliao merged commit 428ce90 into PaddlePaddle:develop Aug 18, 2017
@yu239-zz yu239-zz deleted the cmake branch August 23, 2017 19:10
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