-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Should we use both global use_mkldnn flag and per-OP use_ mkldnn flag as it is in paddle v2? #8313
Comments
global flagWe will have a priority mechanism for kernel-selection, for example, if the default priority is std::vector<std::tuple<platform::Place, LibraryType>> kKernelPriority = {
std::make_tuple(platform::CUDAPlace(0), LibraryType::kCUDNN),
std::make_tuple(platform::CUDAPlace(0), LibraryType::kPlain),
std::make_tuple(platform::CPUPlace(), LibraryType::kMKLDNN),
std::make_tuple(platform::CPUPlace(), LibraryType::kPlain),
} and global use_mkldnn flag will give cudnn a higher priority, if user set it, the priority can be std::vector<std::tuple<platform::Place, LibraryType>> kKernelPriority = {
std::make_tuple(platform::CPUPlace(), LibraryType::kMKLDNN),
std::make_tuple(platform::CUDAPlace(0), LibraryType::kCUDNN),
std::make_tuple(platform::CUDAPlace(0), LibraryType::kPlain),
std::make_tuple(platform::CPUPlace(), LibraryType::kMKLDNN),
std::make_tuple(platform::CPUPlace(), LibraryType::kPlain),
} then the kernel selection will choose mkldnn kernel first if there is one, if there is not one, it will fall back to find another kernel with lower priority. flag in opflag in op attribute is used to force kernel selection to find a kernel of a certain type, if it can not find one, the framework will throw an exception. |
I see that kKernelPriority table is not going to be used until all transformations are implemented: https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/framework/operator.cc#L516 As How does Is |
Ok
kKernelPriority will be a fallback mechanism, the framework will firstly check op flag, if it can not deside then it will use priority to choose a proper kernel.
|
We'd better not set the default value to True now. We should align the accuracy of mkldnn op on PaddlePaddle books Fluid (see https://github.com/dzhwinter/benchmark) at first. |
@luotao1 To align the accuracy of mkldnn op, should we run scripts from https://github.com/dzhwinter/benchmark/tree/master/fluid and check if accuracy does not decrease for MKLDNN kernels? I don't also see any actual results there... Is someone resposible for running benchamarks or developers run them? |
Align the mkldnn operator, I think it can be divided into two parts.
class TestMulOp(OpTest):
def setUp(self):
self.op_type = "mul"
self.inputs = {
'X': np.random.random((32, 84)).astype("float32"),
'Y': np.random.random((84, 100)).astype("float32")
}
self.outputs = {'Out': np.dot(self.inputs['X'], self.inputs['Y'])}
def test_check_output(self):
self.check_output()
def test_check_grad_normal(self):
self.check_grad(['X', 'Y'], 'Out', max_relative_error=0.5) In the forward, we compare the result with numpy.
|
For the benchmark model, you mentioned, sorry for the inconvenient. The benchmark result is organized as below.
Here is the releases notes, we can track the release process at #8533 |
Thank you, @dzhwinter Also, @jczaja ran tests on mnist dataset with MKLDNN kerneles and it converged to the values obtained in caffe framework. @luotao1 I can set |
@pzelazko-intel sorry to reply late. |
This issue comes from Zelazko, Pawel. Related with question #6 in https://github.com/PaddlePaddle/Paddle/wiki/Intel-Open-Questions
In fluid, for CUDNN, GetExpectedKernel decides if it has to use CUDNN library based on operator use_cudnn flag.
How control of kernel-selection is different in unit-test and normal use case?
Having use_mkldnn both as global and OP field, should MLDNN be chosen only when both are set to true, or only the global one?
I’m not sure if I will have an access to global fields in GetExpectedKernel method, I will check that later.
The text was updated successfully, but these errors were encountered: