-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
"remove GPU Sync Interface" #6793
Conversation
if (range.first == range.second) { | ||
PADDLE_THROW( | ||
"'Place' is not supported, Please re-compile with WITH_GPU " | ||
"option"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"..., Please re-compile with WITH_GPU option"
This hint may not be intuitive. Currently platform::Place
contains CUDNNPlace, GPUPlace, CPUPlace, MKLDNNPlace
.
I think we can output this unsupported place by calling this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right.
But we will remove the MKLPlace, CUDNNPlace soon. Place only represent address space, we will split it.
#6746
And, some interface of places isn't supported well. Such as is_cpu_place, default_gpu, we will not fix them because these things will be removed soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
People who look at this code may get confused. And you should add a TODO inside the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am working on removing the Place related problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
fix #6794