Fix a bug in Conv+Activation fusion (#236)

* fix a bug

* fix a bug

* remove node in reverse topologic order.

* replace vector with deque

* fix bugs in conv+activation fusion

* Integrating PR comments.
This commit is contained in:
Du Li 2018-12-31 16:56:11 -08:00 committed by GitHub
parent 4f49a4ab1b
commit 1e9be01a49
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 29 additions and 7 deletions

View file

@ -9,6 +9,7 @@ namespace contrib {
class ONNX_OPERATOR_TYPED_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, float, SampleOp);
class ONNX_OPERATOR_TYPED_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, float, ExpandDims);
class ONNX_OPERATOR_TYPED_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, float, FusedConv);
class ONNX_OPERATOR_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, AttnLSTM);
class ONNX_OPERATOR_TYPED_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, string, Tokenizer);
class ONNX_OPERATOR_TYPED_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, uint8_t, DequantizeLinear);
@ -28,6 +29,7 @@ void RegisterContribKernels(std::function<void(KernelCreateInfo&&)> fn) {
// add more kernels here
fn(BuildKernel<ONNX_OPERATOR_TYPED_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, float, ExpandDims)>());
fn(BuildKernel<ONNX_OPERATOR_TYPED_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, float, FusedConv)>());
fn(BuildKernel<ONNX_OPERATOR_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, AttnLSTM)>());
fn(BuildKernel<ONNX_OPERATOR_TYPED_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, string, Tokenizer)>());
fn(BuildKernel<ONNX_OPERATOR_TYPED_KERNEL_CLASS_NAME(kCpuExecutionProvider, kMSDomain, 1, uint8_t, DequantizeLinear)>());

View file

@ -4,6 +4,7 @@
#include "core/graph/initializer.h"
#include "core/graph/conv_activation_fusion.h"
#include "core/graph/graph_utils.h"
#include <deque>
using namespace onnx;
using namespace ::onnxruntime::common;
@ -13,13 +14,31 @@ namespace {
bool IsFusableActivation(const Node& node) {
return utils::IsSupportedOptypeVersionAndDomain(node, "LeakyRelu", 6) || utils::IsSupportedOptypeVersionAndDomain(node, "Relu", 6) || utils::IsSupportedOptypeVersionAndDomain(node, "Sigmoid", 6) || utils::IsSupportedOptypeVersionAndDomain(node, "Tanh", 6);
}
void HandleActivationNodeEdges(Graph& g, const Node& act, Node& fused_conv) {
Node::EdgeSet output_edges;
for (auto it = act.OutputEdgesBegin(); it != act.OutputEdgesEnd(); ++it) {
output_edges.insert(*it);
}
//remove output edge of activation
//connect fused_conv node and nodes after activation nodes
for (auto& output_edge : output_edges) {
NodeIndex dst_node_index = output_edge.GetNode().Index();
int src_arg_index = output_edge.GetSrcArgIndex();
int dst_arg_index = output_edge.GetDstArgIndex();
g.RemoveEdge(act.Index(), dst_node_index, src_arg_index, dst_arg_index);
g.AddEdge(fused_conv.Index(), dst_node_index, 0, dst_arg_index);
}
}
} // namespace
Status ConvActivationFusion::Apply(Graph& graph, bool& modified) const {
GraphViewer graph_viewer(graph);
const auto& order = graph_viewer.GetNodesInTopologicalOrder();
std::vector<onnxruntime::NodeIndex> removed_nodes;
std::deque<onnxruntime::NodeIndex> removed_nodes;
for (auto index : order) {
auto node = graph.GetNode(index);
if (!utils::IsSupportedOptypeVersionAndDomain(*node, "Conv", 1) || node->GetOutputEdgesCount() != 1) {
@ -32,7 +51,6 @@ Status ConvActivationFusion::Apply(Graph& graph, bool& modified) const {
Node* conv_node = node;
const Node& act_node = next_node;
std::vector<NodeArg> input_args, output_args;
Node& fused_conv = graph.AddNode(graph.GenerateNodeName("fused " + conv_node->Name()), "FusedConv",
"fused Conv " + conv_node->Name() + "with activation " + act_node.OpType(),
@ -42,7 +60,7 @@ Status ConvActivationFusion::Apply(Graph& graph, bool& modified) const {
"com.microsoft");
//Add a new attribute to specify the activation type
fused_conv.AddAttribute("activation", "string");
fused_conv.AddAttribute("activation", act_node.OpType());
//Add optional attributes for activations
if (act_node.OpType() == "LeakyRelu") {
@ -52,6 +70,8 @@ Status ConvActivationFusion::Apply(Graph& graph, bool& modified) const {
}
}
HandleActivationNodeEdges(graph, act_node, fused_conv);
// Replace the input of the node following activation node
const NodeArg* act_output_def = act_node.OutputDefs()[0];
NodeArg* fused_conv_output_def = fused_conv.MutableOutputDefs()[0];
@ -69,12 +89,12 @@ Status ConvActivationFusion::Apply(Graph& graph, bool& modified) const {
}
}
removed_nodes.push_back(act_node.Index());
removed_nodes.push_back(conv_node->Index());
removed_nodes.push_front(conv_node->Index());
removed_nodes.push_front(act_node.Index());
}
for (auto i : removed_nodes) {
graph.RemoveNode(i);
for (auto node : removed_nodes) {
graph.RemoveNode(node);
}
if (!removed_nodes.empty()) {