-
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
Kernel hint design #6790
Kernel hint design #6790
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
## Problem | ||
In PaddlePaddle's [Design](https://github.com/PaddlePaddle/Paddle/blob/develop/doc/design/switch_kernel.md), one Operator may have multiple kernels. Users may have some personal preference to choose a certain type of kernel for an operator, such as `force_cpu` to use a CPU kernel, `use_cudnn` to choose a CUDNN kernel, we need to provide a way for a user to do this. | ||
|
||
In the current design, we use KernelType to describe one kernel. | ||
|
||
```cpp | ||
struct KernelType { | ||
Place place_; | ||
DataType data_type_; | ||
LayoutType layout_; | ||
}; | ||
``` | ||
`place_` `data_type_` and `layout_` can come from the input tensor of the operator, `GetActualKernelType(inputs)` use inputs to infer the proper kernel key that fit the incoming data, user can not config it. | ||
|
||
The design also provides a virtual method `GetExpectedKernelType` that user can overload and choose the KernelType they want to use. | ||
|
||
so, we should send the information user defined in proto to `GetExpectedKernelType` for choosing a kernel. | ||
|
||
The problem is, how should we define and send the information for `GetExpectedKernelType` to use? | ||
|
||
## Solution | ||
|
||
### potential choice | ||
1, Do nothing, let the user add the information they want to operator‘s attribute and get them inside `GetExpectedKernelType`, this can work right. But there is a little problem that users may define many kinds of hints for the same purpose, such as `force_cpu`, `use_cpu`, `CPU` for CPU kernel, and `use_cudnn`, `force_cudnn`, `cudnn_kernel` for use of CUDNN kernel. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can define some global constant for these attribute name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
2, Pre-define all the needed option and use a single attr key such as `kernel_hint` for the user, this is not so flexible if the user wants to define some more kind of hint. | ||
|
||
### final choice | ||
To provide enough flexibility while avoiding confusion definition, we can predefine some options, such as `force_cpu`, `use_cudnn`, `use_mkldnn` for a user to choose. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using string to represent |
||
```cpp | ||
const std::string kNonHint = ""; | ||
const std::string kForceCPU = "force_cpu"; | ||
const std::string kUseCUDNN = "use_cudnn"; | ||
const std::string kUseMKLDNN = "use_mkldnn"; | ||
|
||
KernelType GetExpectedKernelTyp() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
// "kernel_hint" is a user defined attribute name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done, use force_cpu as key and bool as value. |
||
if (Attr<std::string>("kernel_hint") == kForceCPU) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good idea, done |
||
return KernelType(CPUPlace, ...) | ||
} else { | ||
... | ||
} | ||
} | ||
``` | ||
|
||
In Python code | ||
|
||
```python | ||
def xx_layer(..., kernel_hint=None): | ||
layer_helper = ... | ||
layer_helper .append_op( | ||
type="xx", | ||
# "kernel_hint" should be the same with the attr name in CPP | ||
attr={"kernel_hint": kernel_hint or ""}) | ||
``` |
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.
Users cannot configure it.
Actually, users can configure input data and influence the
actual kernel type
.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.
done