You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2021/11/03 13:53:48 UTC
[GitHub] [incubator-mxnet] DominikaJedynak opened a new pull request #20724: Unifying post-quantization properties
DominikaJedynak opened a new pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724
## Description ##Unifying requantize/dequantize post-quantization fuses for different operators into single property.
## Checklist ##
### Essentials ###
- [ ] PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
- [ ] Changes are complete (i.e. I finished coding on this PR)
- [ ] All changes have test coverage
- [ ] Code is well-documented
### Changes ###
- [ ] Feature1, tests, (and when applicable, API doc)
- [ ] Feature2, tests, (and when applicable, API doc)
## Comments ##
- If this change is a backward incompatible change, why must this change be made.
- Interesting edge cases to note here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746524936
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
}
- bool Select(const nnvm::Node& n) override {
- if (n.op() && support_requantize_fusion_op_name.count(n.op()->name)) {
- if (n.op() == Op::Get("_sg_onednn_conv")) {
- auto const& param = nnvm::get<DNNLConvFusionParam>(n.attrs.parsed);
- if (param.full_conv_param.dnnl_param.quantized) {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
- } else if (n.op()->name == "_contrib_quantized_elemwise_add" ||
- n.op()->name == "_contrib_quantized_npi_add") {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
+ bool Select(const BiDirectedNode& n) override {
+ const nnvm::Node* raw_node = n.node;
+ if ((!disable_fuse_all) && raw_node->op() &&
+ support_requantize_fusion_op_name.count(raw_node->op()->name)) {
+ status = kStart;
+ matched_list.clear();
+ matched_list.push_back(&n);
Review comment:
If we clear the vector here, the capacity is the same as before; Then it might be better to use `emplace_back(&n)` instead of `push_back(n)` ~ I hope to avoid creating temp object before.
How about using?
```suggestion
matched_list.emplace_back(&n);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-960520932
Jenkins CI successfully triggered : [centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-977644614
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-959134939
Hey @DominikaJedynak , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:
- To trigger all jobs: @mxnet-bot run ci [all]
- To trigger specific jobs: @mxnet-bot run ci [job1, job2]
***
**CI supported jobs**: [windows-gpu, unix-cpu, sanity, centos-cpu, clang, unix-gpu, miscellaneous, website, windows-cpu, edge, centos-gpu]
***
_Note_:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-959134939
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746427295
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
Review comment:
[Future Consideration] I hope, there is better to replace:
```suggestion
#include "operator/nn/fully_connected-inl.h"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r750705851
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
Review comment:
It was not part of my changes but you are right, it is better to make it private :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-971462164
Jenkins CI successfully triggered : [centos-cpu, miscellaneous, unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746457634
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
Review comment:
What are the pros and cons of using this assignment? Roughly, why the set is global and then it's inexplicably assigned?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746524936
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
}
- bool Select(const nnvm::Node& n) override {
- if (n.op() && support_requantize_fusion_op_name.count(n.op()->name)) {
- if (n.op() == Op::Get("_sg_onednn_conv")) {
- auto const& param = nnvm::get<DNNLConvFusionParam>(n.attrs.parsed);
- if (param.full_conv_param.dnnl_param.quantized) {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
- } else if (n.op()->name == "_contrib_quantized_elemwise_add" ||
- n.op()->name == "_contrib_quantized_npi_add") {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
+ bool Select(const BiDirectedNode& n) override {
+ const nnvm::Node* raw_node = n.node;
+ if ((!disable_fuse_all) && raw_node->op() &&
+ support_requantize_fusion_op_name.count(raw_node->op()->name)) {
+ status = kStart;
+ matched_list.clear();
+ matched_list.push_back(&n);
Review comment:
If we clear the vector here, the capacity is the same as before; Then it looks better to use matched_list.emplace_back(&n) ~ I hope to avoid creating temp object before. If you know, how many values you have, then you can reserve memory before: to avoid doing 2^{N}.
How about using?
```suggestion
matched_list.emplace_back(&n);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r750712116
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
}
- bool Select(const nnvm::Node& n) override {
- if (n.op() && support_requantize_fusion_op_name.count(n.op()->name)) {
- if (n.op() == Op::Get("_sg_onednn_conv")) {
- auto const& param = nnvm::get<DNNLConvFusionParam>(n.attrs.parsed);
- if (param.full_conv_param.dnnl_param.quantized) {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
- } else if (n.op()->name == "_contrib_quantized_elemwise_add" ||
- n.op()->name == "_contrib_quantized_npi_add") {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
+ bool Select(const BiDirectedNode& n) override {
+ const nnvm::Node* raw_node = n.node;
+ if ((!disable_fuse_all) && raw_node->op() &&
+ support_requantize_fusion_op_name.count(raw_node->op()->name)) {
+ status = kStart;
+ matched_list.clear();
+ matched_list.push_back(&n);
+ return true;
}
return false;
}
- bool SelectInput(const nnvm::Node& n, const nnvm::Node& new_node) override {
+ bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& new_node) override {
return false;
}
- bool SelectOutput(const nnvm::Node& n, const nnvm::Node& new_node) override {
- if (status == kFail || status == kSuccess || new_node.is_variable())
+ bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& new_node) override {
+ const nnvm::Node* raw_node = n.node;
+ const nnvm::Node* raw_new_node = new_node.node;
+ if (status == kFail || status == kSuccess || raw_new_node->is_variable())
return false;
// If n isn't the last matched node, then we encoutered a internal
// branch, we should pop out the node behind n and stop fusion.
if (matched_list.back() != &n) {
- status = kFail;
+ if (std::find(matched_list.begin(), matched_list.end(), &n) != matched_list.end()) {
+ while (matched_list.back() != &n) {
+ matched_list.pop_back();
+ }
+ }
+ status = kSuccess;
return false;
}
- if (new_node.op()->name == "_contrib_requantize") {
- auto const& param = nnvm::get<RequantizeParam>(new_node.attrs.parsed);
- if (param.min_calib_range.has_value() && param.max_calib_range.has_value()) {
- matched_list.push_back(&new_node);
+
+ switch (status) {
+ case kStart:
+ if (raw_new_node->op() == Op::Get("_contrib_requantize")) {
+ auto const& param = nnvm::get<RequantizeParam>(raw_new_node->attrs.parsed);
+ if (param.min_calib_range.has_value() && param.max_calib_range.has_value()) {
+ matched_list.push_back(&new_node);
+ status = kRequantize;
+ if (raw_node->op() == Op::Get("_sg_onednn_conv")) {
+ status = kSuccess;
+ }
+ return true;
+ }
+ }
+ case kRequantize:
+ if (!disable_float_output && raw_new_node->op() == Op::Get("_contrib_dequantize")) {
+ CHECK(raw_node->op() == Op::Get("_contrib_requantize"));
+ if (n.outputs.size() > 1) {
+ // check if requantize have other outputs than dequantize
+ // if it has we can't fuse dequantize
+ for (auto kv : n.outputs) {
Review comment:
kv.first, which is the only place where kv is used, is marked const, but I can make kv const as well for clarity
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-959823169
Jenkins CI successfully triggered : [unix-cpu, centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-959134939
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746428664
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
Review comment:
See above
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
Review comment:
[Future consideration] + @anko-intel
```suggestion
#include "./dnnl_convolution-inl.h"
```
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
Review comment:
[Future Consideration] I hope, there is better to replace:
```suggestion
#include "operator/nn/fully_connected-inl.h"
```
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
Review comment:
This global set is likely enabled in all TU. What is an advantage of using it in this place?
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
Review comment:
What are the pros and cons of using this assignment? Roughly, why the set is global and then inexplicitly assigned?
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
}
- bool Select(const nnvm::Node& n) override {
- if (n.op() && support_requantize_fusion_op_name.count(n.op()->name)) {
- if (n.op() == Op::Get("_sg_onednn_conv")) {
- auto const& param = nnvm::get<DNNLConvFusionParam>(n.attrs.parsed);
- if (param.full_conv_param.dnnl_param.quantized) {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
- } else if (n.op()->name == "_contrib_quantized_elemwise_add" ||
- n.op()->name == "_contrib_quantized_npi_add") {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
+ bool Select(const BiDirectedNode& n) override {
+ const nnvm::Node* raw_node = n.node;
+ if ((!disable_fuse_all) && raw_node->op() &&
+ support_requantize_fusion_op_name.count(raw_node->op()->name)) {
+ status = kStart;
+ matched_list.clear();
+ matched_list.push_back(&n);
+ return true;
}
return false;
}
- bool SelectInput(const nnvm::Node& n, const nnvm::Node& new_node) override {
+ bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& new_node) override {
return false;
}
- bool SelectOutput(const nnvm::Node& n, const nnvm::Node& new_node) override {
- if (status == kFail || status == kSuccess || new_node.is_variable())
+ bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& new_node) override {
+ const nnvm::Node* raw_node = n.node;
+ const nnvm::Node* raw_new_node = new_node.node;
+ if (status == kFail || status == kSuccess || raw_new_node->is_variable())
return false;
// If n isn't the last matched node, then we encoutered a internal
// branch, we should pop out the node behind n and stop fusion.
if (matched_list.back() != &n) {
- status = kFail;
+ if (std::find(matched_list.begin(), matched_list.end(), &n) != matched_list.end()) {
+ while (matched_list.back() != &n) {
+ matched_list.pop_back();
+ }
+ }
+ status = kSuccess;
return false;
}
- if (new_node.op()->name == "_contrib_requantize") {
- auto const& param = nnvm::get<RequantizeParam>(new_node.attrs.parsed);
- if (param.min_calib_range.has_value() && param.max_calib_range.has_value()) {
- matched_list.push_back(&new_node);
+
+ switch (status) {
+ case kStart:
+ if (raw_new_node->op() == Op::Get("_contrib_requantize")) {
+ auto const& param = nnvm::get<RequantizeParam>(raw_new_node->attrs.parsed);
+ if (param.min_calib_range.has_value() && param.max_calib_range.has_value()) {
+ matched_list.push_back(&new_node);
+ status = kRequantize;
+ if (raw_node->op() == Op::Get("_sg_onednn_conv")) {
+ status = kSuccess;
+ }
+ return true;
+ }
+ }
+ case kRequantize:
+ if (!disable_float_output && raw_new_node->op() == Op::Get("_contrib_dequantize")) {
+ CHECK(raw_node->op() == Op::Get("_contrib_requantize"));
+ if (n.outputs.size() > 1) {
+ // check if requantize have other outputs than dequantize
+ // if it has we can't fuse dequantize
+ for (auto kv : n.outputs) {
Review comment:
How about? If an object is const-> then `kv` is const, otherwise, the `kv` might be non-const.
```suggestion
for (const auto kv : n.outputs) {
```
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
Review comment:
[+] It could be fine to replace plain enum with class enum - to avoid implicitly conversion to other types (like another enum or int)
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
Review comment:
Why the enum (`enum SelectStatus`) is public?
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
}
- bool Select(const nnvm::Node& n) override {
- if (n.op() && support_requantize_fusion_op_name.count(n.op()->name)) {
- if (n.op() == Op::Get("_sg_onednn_conv")) {
- auto const& param = nnvm::get<DNNLConvFusionParam>(n.attrs.parsed);
- if (param.full_conv_param.dnnl_param.quantized) {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
- } else if (n.op()->name == "_contrib_quantized_elemwise_add" ||
- n.op()->name == "_contrib_quantized_npi_add") {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
+ bool Select(const BiDirectedNode& n) override {
+ const nnvm::Node* raw_node = n.node;
+ if ((!disable_fuse_all) && raw_node->op() &&
+ support_requantize_fusion_op_name.count(raw_node->op()->name)) {
+ status = kStart;
+ matched_list.clear();
+ matched_list.push_back(&n);
Review comment:
If we clear the vector here, the capacity is the same as before; Then it looks better to use matched_list.emplace_back(&n) to avoid creating temp object before. If you know, how many values you have, then you can reserve memory before: to avoid doing 2^{N}.
How about using?
```suggestion
matched_list.emplace_back(&n);
```
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
Review comment:
[Consideration]
```suggestion
bool disable_fuse_all{false or true};
```
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
Review comment:
What are the pros of using `const bool` here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-959823094
@mxnet-bot run ci [centos-gpu, unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-964540488
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746524936
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
}
- bool Select(const nnvm::Node& n) override {
- if (n.op() && support_requantize_fusion_op_name.count(n.op()->name)) {
- if (n.op() == Op::Get("_sg_onednn_conv")) {
- auto const& param = nnvm::get<DNNLConvFusionParam>(n.attrs.parsed);
- if (param.full_conv_param.dnnl_param.quantized) {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
- } else if (n.op()->name == "_contrib_quantized_elemwise_add" ||
- n.op()->name == "_contrib_quantized_npi_add") {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
+ bool Select(const BiDirectedNode& n) override {
+ const nnvm::Node* raw_node = n.node;
+ if ((!disable_fuse_all) && raw_node->op() &&
+ support_requantize_fusion_op_name.count(raw_node->op()->name)) {
+ status = kStart;
+ matched_list.clear();
+ matched_list.push_back(&n);
Review comment:
If we clear the vector here, the capacity is the same as before; Then it looks better to use matched_list.emplace_back(&n) ~ I hope to avoid creating temp object before.
How about using?
```suggestion
matched_list.emplace_back(&n);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] bgawrych commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
bgawrych commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746318695
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
}
- bool Select(const nnvm::Node& n) override {
- if (n.op() && support_requantize_fusion_op_name.count(n.op()->name)) {
- if (n.op() == Op::Get("_sg_onednn_conv")) {
- auto const& param = nnvm::get<DNNLConvFusionParam>(n.attrs.parsed);
- if (param.full_conv_param.dnnl_param.quantized) {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
- } else if (n.op()->name == "_contrib_quantized_elemwise_add" ||
- n.op()->name == "_contrib_quantized_npi_add") {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
+ bool Select(const BiDirectedNode& n) override {
+ const nnvm::Node* raw_node = n.node;
+ if ((!disable_fuse_all) && raw_node->op() &&
+ support_requantize_fusion_op_name.count(raw_node->op()->name)) {
+ status = kStart;
+ matched_list.clear();
+ matched_list.push_back(&n);
+ return true;
}
return false;
}
- bool SelectInput(const nnvm::Node& n, const nnvm::Node& new_node) override {
+ bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& new_node) override {
return false;
}
- bool SelectOutput(const nnvm::Node& n, const nnvm::Node& new_node) override {
- if (status == kFail || status == kSuccess || new_node.is_variable())
+ bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& new_node) override {
+ const nnvm::Node* raw_node = n.node;
+ const nnvm::Node* raw_new_node = new_node.node;
+ if (status == kFail || status == kSuccess || raw_new_node->is_variable())
return false;
// If n isn't the last matched node, then we encoutered a internal
// branch, we should pop out the node behind n and stop fusion.
if (matched_list.back() != &n) {
- status = kFail;
+ if (std::find(matched_list.begin(), matched_list.end(), &n) != matched_list.end()) {
+ while (matched_list.back() != &n) {
+ matched_list.pop_back();
+ }
+ }
+ status = kSuccess;
return false;
}
- if (new_node.op()->name == "_contrib_requantize") {
- auto const& param = nnvm::get<RequantizeParam>(new_node.attrs.parsed);
- if (param.min_calib_range.has_value() && param.max_calib_range.has_value()) {
- matched_list.push_back(&new_node);
+
+ switch (status) {
+ case kStart:
+ if (raw_new_node->op() == Op::Get("_contrib_requantize")) {
+ auto const& param = nnvm::get<RequantizeParam>(raw_new_node->attrs.parsed);
+ if (param.min_calib_range.has_value() && param.max_calib_range.has_value()) {
+ matched_list.push_back(&new_node);
+ status = kRequantize;
+ if (raw_node->op() == Op::Get("_sg_onednn_conv")) {
+ status = kSuccess;
+ }
+ return true;
+ }
+ }
+ case kRequantize:
+ if (!disable_float_output && raw_new_node->op() == Op::Get("_contrib_dequantize")) {
+ CHECK(raw_node->op() == Op::Get("_contrib_requantize"));
+ if (n.outputs.size() > 1) {
+ // check if requantize have other outputs than dequantize
+ // if it has we can't fuse dequantize
+ for (auto kv : n.outputs) {
+ const auto& node = kv.first;
+ if (node->op() != Op::Get("_contrib_dequantize")) {
+ status = kSuccess;
+ return false;
+ }
+ }
+ }
+ matched_list.push_back(&new_node);
+ status = kSuccess;
+ return true;
+ }
+ default:
status = kSuccess;
- return true;
- } else {
- status = kFail;
- }
+ return false;
}
- return false;
}
- std::vector<nnvm::Node*> Filter(const std::vector<nnvm::Node*>& candidates) override {
- if (status != kSuccess) {
- return std::vector<nnvm::Node*>(0);
+ std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+ if (status != kSuccess || (matched_list.size() <= 1)) {
+ return std::vector<BiDirectedNode*>(0);
} else {
- return candidates;
+ std::vector<BiDirectedNode*> ret;
+ for (auto i : matched_list) {
+ auto non_const_i = const_cast<BiDirectedNode*>(i);
+ if (std::find(candidates.begin(), candidates.end(), non_const_i) != candidates.end()) {
+ ret.push_back(non_const_i);
+ }
+ }
+ return ret;
}
}
void Reset() override {
CHECK_GE(matched_list.size(), 1);
- auto new_selector = SgDNNLPostQuantizeSelector();
+ auto new_selector = SgDNNLPostQuantizeSelector(disable_fuse_all, disable_float_output);
new_selector.Select(*matched_list[0]);
*this = new_selector;
}
};
class SgDNNLPostQuantizeProperty : public SubgraphProperty {
+ private:
+ bool disable_fuse_all;
+ bool disable_float_output;
+ std::set<std::string> support_requantize_fusion_op_name;
+
public:
SgDNNLPostQuantizeProperty() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ disable_fuse_all = dmlc::GetEnv("MXNET_DISABLE_ONEDNN_FUSE_ALL", false);
Review comment:
What about changing this flag to something less general? I mean this flag doesn't disable all fuses (only the ones in this file). So better would be something like "MXNET_DISABLE_ONEDNN_FUSE_REQUANTIZE" and "MXNET_DISABLE_ONEDNN_FUSE_DEQUANTIZE". We also can negate default value to call it "MXNET_ONEDNN_FUSE_REQUANTIZE" and "MXNET_ONEDNN_FUSE_DEQUANTIZE".
+@anko-intel
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r750708825
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
Review comment:
It is used in two different classes and is private in both, and I wanted to avoid duplicating. I will place this set inside anonymous namespace, as we discussed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r750704638
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
Review comment:
This particular file is in nn/dnnl, not subgraph/dnnl, but I can change all paths in a way that avoids dots if you consider it more transparent
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-977259070
Jenkins CI successfully triggered : [unix-cpu, miscellaneous]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-977644565
@mxnet-bot run ci [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r744798383
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
Review comment:
[Future consideration] It could be fine to replace plain enum with class enum - to avoid implicitly conversion to other types (like another enum or int)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] bgawrych commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
bgawrych commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746318695
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
}
- bool Select(const nnvm::Node& n) override {
- if (n.op() && support_requantize_fusion_op_name.count(n.op()->name)) {
- if (n.op() == Op::Get("_sg_onednn_conv")) {
- auto const& param = nnvm::get<DNNLConvFusionParam>(n.attrs.parsed);
- if (param.full_conv_param.dnnl_param.quantized) {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
- } else if (n.op()->name == "_contrib_quantized_elemwise_add" ||
- n.op()->name == "_contrib_quantized_npi_add") {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
+ bool Select(const BiDirectedNode& n) override {
+ const nnvm::Node* raw_node = n.node;
+ if ((!disable_fuse_all) && raw_node->op() &&
+ support_requantize_fusion_op_name.count(raw_node->op()->name)) {
+ status = kStart;
+ matched_list.clear();
+ matched_list.push_back(&n);
+ return true;
}
return false;
}
- bool SelectInput(const nnvm::Node& n, const nnvm::Node& new_node) override {
+ bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& new_node) override {
return false;
}
- bool SelectOutput(const nnvm::Node& n, const nnvm::Node& new_node) override {
- if (status == kFail || status == kSuccess || new_node.is_variable())
+ bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& new_node) override {
+ const nnvm::Node* raw_node = n.node;
+ const nnvm::Node* raw_new_node = new_node.node;
+ if (status == kFail || status == kSuccess || raw_new_node->is_variable())
return false;
// If n isn't the last matched node, then we encoutered a internal
// branch, we should pop out the node behind n and stop fusion.
if (matched_list.back() != &n) {
- status = kFail;
+ if (std::find(matched_list.begin(), matched_list.end(), &n) != matched_list.end()) {
+ while (matched_list.back() != &n) {
+ matched_list.pop_back();
+ }
+ }
+ status = kSuccess;
return false;
}
- if (new_node.op()->name == "_contrib_requantize") {
- auto const& param = nnvm::get<RequantizeParam>(new_node.attrs.parsed);
- if (param.min_calib_range.has_value() && param.max_calib_range.has_value()) {
- matched_list.push_back(&new_node);
+
+ switch (status) {
+ case kStart:
+ if (raw_new_node->op() == Op::Get("_contrib_requantize")) {
+ auto const& param = nnvm::get<RequantizeParam>(raw_new_node->attrs.parsed);
+ if (param.min_calib_range.has_value() && param.max_calib_range.has_value()) {
+ matched_list.push_back(&new_node);
+ status = kRequantize;
+ if (raw_node->op() == Op::Get("_sg_onednn_conv")) {
+ status = kSuccess;
+ }
+ return true;
+ }
+ }
+ case kRequantize:
+ if (!disable_float_output && raw_new_node->op() == Op::Get("_contrib_dequantize")) {
+ CHECK(raw_node->op() == Op::Get("_contrib_requantize"));
+ if (n.outputs.size() > 1) {
+ // check if requantize have other outputs than dequantize
+ // if it has we can't fuse dequantize
+ for (auto kv : n.outputs) {
+ const auto& node = kv.first;
+ if (node->op() != Op::Get("_contrib_dequantize")) {
+ status = kSuccess;
+ return false;
+ }
+ }
+ }
+ matched_list.push_back(&new_node);
+ status = kSuccess;
+ return true;
+ }
+ default:
status = kSuccess;
- return true;
- } else {
- status = kFail;
- }
+ return false;
}
- return false;
}
- std::vector<nnvm::Node*> Filter(const std::vector<nnvm::Node*>& candidates) override {
- if (status != kSuccess) {
- return std::vector<nnvm::Node*>(0);
+ std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+ if (status != kSuccess || (matched_list.size() <= 1)) {
+ return std::vector<BiDirectedNode*>(0);
} else {
- return candidates;
+ std::vector<BiDirectedNode*> ret;
+ for (auto i : matched_list) {
+ auto non_const_i = const_cast<BiDirectedNode*>(i);
+ if (std::find(candidates.begin(), candidates.end(), non_const_i) != candidates.end()) {
+ ret.push_back(non_const_i);
+ }
+ }
+ return ret;
}
}
void Reset() override {
CHECK_GE(matched_list.size(), 1);
- auto new_selector = SgDNNLPostQuantizeSelector();
+ auto new_selector = SgDNNLPostQuantizeSelector(disable_fuse_all, disable_float_output);
new_selector.Select(*matched_list[0]);
*this = new_selector;
}
};
class SgDNNLPostQuantizeProperty : public SubgraphProperty {
+ private:
+ bool disable_fuse_all;
+ bool disable_float_output;
+ std::set<std::string> support_requantize_fusion_op_name;
+
public:
SgDNNLPostQuantizeProperty() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ disable_fuse_all = dmlc::GetEnv("MXNET_DISABLE_ONEDNN_FUSE_ALL", false);
Review comment:
What about changing this flag to something less general? I mean this flag doesn't disable all fuses (only in this file). So better would be something like "MXNET_DISABLE_ONEDNN_FUSE_REQUANTIZE" and "MXNET_DISABLE_ONEDNN_FUSE_DEQUANTIZE". We also can negate default value to call it "MXNET_ONEDNN_FUSE_REQUANTIZE" and "MXNET_ONEDNN_FUSE_DEQUANTIZE".
+@anko-intel
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r750710186
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
Review comment:
Commented above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-964540388
@mxnet-bot run ci [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r745660644
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
Review comment:
What are the pros of using `const bool` here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746428664
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
Review comment:
See above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746430081
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
Review comment:
[Future consideration] Why the enum (`enum SelectStatus`) is public?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] bgawrych merged pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
bgawrych merged pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-978301622
@mxnet-bot run ci [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-959823094
@mxnet-bot run ci [centos-gpu, unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746444188
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
Review comment:
[Consideration]
```suggestion
bool disable_fuse_all{false or true};
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] bgawrych commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
bgawrych commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-971462110
@mxnet-bot run ci [unix-cpu, centos-cpu, miscellaneous ]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-960520882
@mxnet-bot run ci [centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-959823094
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-959134939
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] anko-intel commented on a change in pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
anko-intel commented on a change in pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#discussion_r746361426
##########
File path: src/operator/subgraph/dnnl/dnnl_post_quantize_property.h
##########
@@ -20,146 +20,209 @@
#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_POST_QUANTIZE_PROPERTY_H_
#if MXNET_USE_ONEDNN == 1
+#include <memory>
#include <set>
#include <string>
#include <vector>
#include "../../nn/dnnl/dnnl_convolution-inl.h"
+#include "../../nn/fully_connected-inl.h"
#include "../../quantization/requantize-inl.h"
+#include "../../tensor/elemwise_binary_op-inl.h"
#include "../common.h"
#include "dnnl_conv-inl.h"
#include "dnnl_subgraph_base-inl.h"
namespace mxnet {
namespace op {
-class SgDNNLPostQuantizeSelector : public SubgraphSelector {
+const std::set<std::string> support_req_fusion_op = {"_contrib_quantized_elemwise_add",
+ "_contrib_quantized_elemwise_mul",
+ "_contrib_quantized_npi_add",
+ "_sg_onednn_conv",
+ "_sg_onednn_fully_connected",
+ "_sg_onednn_selfatt_qk",
+ "_sg_onednn_selfatt_valatt",
+ "_sg_onednn_batch_dot"};
+
+class SgDNNLPostQuantizeSelector : public SubgraphSelectorV2 {
public:
/*! \brief pattern match status */
enum SelectStatus {
kFail = 0,
kStart,
+ kRequantize,
kSuccess,
};
private:
+ bool disable_fuse_all;
+ bool disable_float_output;
SelectStatus status;
- std::vector<const nnvm::Node*> matched_list;
+ std::vector<const BiDirectedNode*> matched_list;
std::set<std::string> support_requantize_fusion_op_name;
public:
- SgDNNLPostQuantizeSelector() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ explicit SgDNNLPostQuantizeSelector(const bool dis_fuse_all, const bool dis_float_output)
+ : disable_fuse_all(dis_fuse_all), disable_float_output(dis_float_output) {
+ support_requantize_fusion_op_name = support_req_fusion_op;
}
- bool Select(const nnvm::Node& n) override {
- if (n.op() && support_requantize_fusion_op_name.count(n.op()->name)) {
- if (n.op() == Op::Get("_sg_onednn_conv")) {
- auto const& param = nnvm::get<DNNLConvFusionParam>(n.attrs.parsed);
- if (param.full_conv_param.dnnl_param.quantized) {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
- } else if (n.op()->name == "_contrib_quantized_elemwise_add" ||
- n.op()->name == "_contrib_quantized_npi_add") {
- status = kStart;
- matched_list.clear();
- matched_list.push_back(&n);
- return true;
- }
+ bool Select(const BiDirectedNode& n) override {
+ const nnvm::Node* raw_node = n.node;
+ if ((!disable_fuse_all) && raw_node->op() &&
+ support_requantize_fusion_op_name.count(raw_node->op()->name)) {
+ status = kStart;
+ matched_list.clear();
+ matched_list.push_back(&n);
+ return true;
}
return false;
}
- bool SelectInput(const nnvm::Node& n, const nnvm::Node& new_node) override {
+ bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& new_node) override {
return false;
}
- bool SelectOutput(const nnvm::Node& n, const nnvm::Node& new_node) override {
- if (status == kFail || status == kSuccess || new_node.is_variable())
+ bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& new_node) override {
+ const nnvm::Node* raw_node = n.node;
+ const nnvm::Node* raw_new_node = new_node.node;
+ if (status == kFail || status == kSuccess || raw_new_node->is_variable())
return false;
// If n isn't the last matched node, then we encoutered a internal
// branch, we should pop out the node behind n and stop fusion.
if (matched_list.back() != &n) {
- status = kFail;
+ if (std::find(matched_list.begin(), matched_list.end(), &n) != matched_list.end()) {
+ while (matched_list.back() != &n) {
+ matched_list.pop_back();
+ }
+ }
+ status = kSuccess;
return false;
}
- if (new_node.op()->name == "_contrib_requantize") {
- auto const& param = nnvm::get<RequantizeParam>(new_node.attrs.parsed);
- if (param.min_calib_range.has_value() && param.max_calib_range.has_value()) {
- matched_list.push_back(&new_node);
+
+ switch (status) {
+ case kStart:
+ if (raw_new_node->op() == Op::Get("_contrib_requantize")) {
+ auto const& param = nnvm::get<RequantizeParam>(raw_new_node->attrs.parsed);
+ if (param.min_calib_range.has_value() && param.max_calib_range.has_value()) {
+ matched_list.push_back(&new_node);
+ status = kRequantize;
+ if (raw_node->op() == Op::Get("_sg_onednn_conv")) {
+ status = kSuccess;
+ }
+ return true;
+ }
+ }
+ case kRequantize:
+ if (!disable_float_output && raw_new_node->op() == Op::Get("_contrib_dequantize")) {
+ CHECK(raw_node->op() == Op::Get("_contrib_requantize"));
+ if (n.outputs.size() > 1) {
+ // check if requantize have other outputs than dequantize
+ // if it has we can't fuse dequantize
+ for (auto kv : n.outputs) {
+ const auto& node = kv.first;
+ if (node->op() != Op::Get("_contrib_dequantize")) {
+ status = kSuccess;
+ return false;
+ }
+ }
+ }
+ matched_list.push_back(&new_node);
+ status = kSuccess;
+ return true;
+ }
+ default:
status = kSuccess;
- return true;
- } else {
- status = kFail;
- }
+ return false;
}
- return false;
}
- std::vector<nnvm::Node*> Filter(const std::vector<nnvm::Node*>& candidates) override {
- if (status != kSuccess) {
- return std::vector<nnvm::Node*>(0);
+ std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+ if (status != kSuccess || (matched_list.size() <= 1)) {
+ return std::vector<BiDirectedNode*>(0);
} else {
- return candidates;
+ std::vector<BiDirectedNode*> ret;
+ for (auto i : matched_list) {
+ auto non_const_i = const_cast<BiDirectedNode*>(i);
+ if (std::find(candidates.begin(), candidates.end(), non_const_i) != candidates.end()) {
+ ret.push_back(non_const_i);
+ }
+ }
+ return ret;
}
}
void Reset() override {
CHECK_GE(matched_list.size(), 1);
- auto new_selector = SgDNNLPostQuantizeSelector();
+ auto new_selector = SgDNNLPostQuantizeSelector(disable_fuse_all, disable_float_output);
new_selector.Select(*matched_list[0]);
*this = new_selector;
}
};
class SgDNNLPostQuantizeProperty : public SubgraphProperty {
+ private:
+ bool disable_fuse_all;
+ bool disable_float_output;
+ std::set<std::string> support_requantize_fusion_op_name;
+
public:
SgDNNLPostQuantizeProperty() {
- support_requantize_fusion_op_name.insert("_sg_onednn_conv");
- support_requantize_fusion_op_name.insert("_contrib_quantized_elemwise_add");
- support_requantize_fusion_op_name.insert("_contrib_quantized_npi_add");
+ disable_fuse_all = dmlc::GetEnv("MXNET_DISABLE_ONEDNN_FUSE_ALL", false);
Review comment:
Fully agree. "MXNET_ONEDNN_FUSE_REQUANTIZE" and "MXNET_ONEDNN_FUSE_DEQUANTIZE" are good proposals
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] DominikaJedynak commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-977258973
@mxnet-bot run ci [unix-cpu, miscellaneous ]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20724: Unifying post-quantization properties
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20724:
URL: https://github.com/apache/incubator-mxnet/pull/20724#issuecomment-978301828
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org