{"action":"created","issue":{"url":"https://api.github.com/repos/PaddlePaddle/Paddle/issues/10291","repository_url":"https://api.github.com/repos/PaddlePaddle/Paddle","labels_url":"https://api.github.com/repos/PaddlePaddle/Paddle/issues/10291/labels{/name}","comments_url":"https://api.github.com/repos/PaddlePaddle/Paddle/issues/10291/comments","events_url":"https://api.github.com/repos/PaddlePaddle/Paddle/issues/10291/events","html_url":"https://github.com/PaddlePaddle/Paddle/pull/10291","id":318626432,"number":10291,"title":"[DO NOT MERGE] MKLDNN layouts","user":{"login":"pzelazko-intel","id":20774899,"avatar_url":"https://avatars3.githubusercontent.com/u/20774899?v=4","gravatar_id":"","url":"https://api.github.com/users/pzelazko-intel","html_url":"https://github.com/pzelazko-intel","followers_url":"https://api.github.com/users/pzelazko-intel/followers","following_url":"https://api.github.com/users/pzelazko-intel/following{/other_user}","gists_url":"https://api.github.com/users/pzelazko-intel/gists{/gist_id}","starred_url":"https://api.github.com/users/pzelazko-intel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pzelazko-intel/subscriptions","organizations_url":"https://api.github.com/users/pzelazko-intel/orgs","repos_url":"https://api.github.com/users/pzelazko-intel/repos","events_url":"https://api.github.com/users/pzelazko-intel/events{/privacy}","received_events_url":"https://api.github.com/users/pzelazko-intel/received_events","type":"User","site_admin":false},"labels":[{"id":831909412,"url":"https://api.github.com/repos/PaddlePaddle/Paddle/labels/Intel","name":"Intel","color":"53a9d1","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":21,"created_at":"2018-04-28T11:04:05Z","updated_at":"2018-05-11T07:19:57Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/PaddlePaddle/Paddle/pulls/10291","html_url":"https://github.com/PaddlePaddle/Paddle/pull/10291","diff_url":"https://github.com/PaddlePaddle/Paddle/pull/10291.diff","patch_url":"https://github.com/PaddlePaddle/Paddle/pull/10291.patch"},"body":"**DO NOT MERGE IT**\r\n\r\nPlease take a look at MKLDNN layout  Proof of Concept and assess if we can stay with this design. This code is not finished - there are debug VLOG prints, some parts are missing and the code needs cleanup.\r\n\r\nWe need MKLDNN layouts for MKLDNN kernels to be performed efficiently - especially for convolution and fully-connected OPs.\r\n\r\nIn #8305 it was recommended to create `MKLDNNLoDTensor` class deriving from `LoDTensor`. But that approach was causing too many problems - in many places `Tensor` class is used explicitly and I would need to differentiate between `LoDTensor` and `MKLDNNLoDTensor`.\r\n\r\nI came up with a different approach - I've added `Tensor::ExtendedData` interface and a pointer to this interface in `Tensor` class. It is a placeholder for additional data. I use it for MKLDNN layout case - `mkldnn::memory::format` and `mkldnn::engine` are held in `MKLDNNTensorData`, which derives from `Tensor::ExtendedData`. If `Tensor` layout is set to `kMKLDNN`, then I can treat this tensor as holding data in MKLDNN format layout. To make use of such a tensor, I've created decorator classes `MKLDNNTensor` and `MutableMKLDNNTensor`. The data is kept still in `Placeholder` within `Tensor`.\r\n\r\nReorders to MKLDNN format happen within convolution and fully-connected, because:\r\n* For these two OPs MKLDNN library  can decide which layout is best based on input parameters like the size of the input, stride, padding etc.\r\n* Other MKLDNN OPs don't offer significant performance boost on MKLDNN layouts\r\n\r\nWhen the next OP expected kernel is not a MKLDNN one, then we transform tensor to `NCHW` layout."},"comment":{"url":"https://api.github.com/repos/PaddlePaddle/Paddle/issues/comments/388282615","html_url":"https://github.com/PaddlePaddle/Paddle/pull/10291#issuecomment-388282615","issue_url":"https://api.github.com/repos/PaddlePaddle/Paddle/issues/10291","id":388282615,"user":{"login":"jacquesqiao","id":3048612,"avatar_url":"https://avatars0.githubusercontent.com/u/3048612?v=4","gravatar_id":"","url":"https://api.github.com/users/jacquesqiao","html_url":"https://github.com/jacquesqiao","followers_url":"https://api.github.com/users/jacquesqiao/followers","following_url":"https://api.github.com/users/jacquesqiao/following{/other_user}","gists_url":"https://api.github.com/users/jacquesqiao/gists{/gist_id}","starred_url":"https://api.github.com/users/jacquesqiao/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jacquesqiao/subscriptions","organizations_url":"https://api.github.com/users/jacquesqiao/orgs","repos_url":"https://api.github.com/users/jacquesqiao/repos","events_url":"https://api.github.com/users/jacquesqiao/events{/privacy}","received_events_url":"https://api.github.com/users/jacquesqiao/received_events","type":"User","site_admin":false},"created_at":"2018-05-11T07:19:57Z","updated_at":"2018-05-11T07:19:57Z","author_association":"CONTRIBUTOR","body":"@pzelazko-intel After data transform, we will copy the transformed data inside tensor back to it's original var type, the lod infromation will also be copied.\r\n\r\nhttps://github.com/PaddlePaddle/Paddle/blob/18f527bfa35cd7dabdd7abc9d370f55650cfa38e/paddle/fluid/framework/operator.cc#L574-L578\r\nhttps://github.com/PaddlePaddle/Paddle/blob/f09aed047575d9cb97c3e2297df33799c4af72fb/paddle/fluid/framework/data_transform.cc#L65-L81\r\n\r\nIf we add MKLDNNLoDTensor, we also need to do this, and the LoD information should maintain in the MKLDNNLoDTensor. That is why we think MKLDNNLoDTensor should derive from LoDTensor, because it need to store the LoD information inside it."}}
{"actor":{"display_login":"jacquesqiao"}}