bartekkuncer commented on a change in pull request #20393:
URL: https://github.com/apache/incubator-mxnet/pull/20393#discussion_r659668745
##########
File path: src/operator/subgraph/mkldnn/mkldnn_fc_property.h
##########
@@ -113,13 +113,17 @@ class SgMKLDNNFCSelector : public SubgraphSelector {
}
}
if (!quantized_ && (new_node.op() == Op::Get("square") ||
+ new_node.op() == Op::Get("_npi_square") ||
new_node.op() == Op::Get("sqrt") ||
- new_node.op() == Op::Get("exp"))) {
+ new_node.op() == Op::Get("_npi_sqrt") ||
+ new_node.op() == Op::Get("exp") ||
+ new_node.op() == Op::Get("_npi_exp"))) {
matched_list_.push_back(&new_node);
status_ = kSuccess;
return true;
}
- if (new_node.op() == Op::Get("abs")) {
+ if (new_node.op() == Op::Get("abs") ||
+ new_node.op() == Op::Get("_npi_absolute")) {
Review comment:
If you will be changing something I think it would be nice to put those
checks in pairs e.g. `if (new_node.op() == Op::Get("abs") || new_node.op() ==
Op::Get("_npi_absolute")) {` or `new_node.op() == Op::Get("exp") ||
new_node.op() == Op::Get("_npi_exp")`.
##########
File path: tests/python/mkl/subgraphs/test_fc_subgraph.py
##########
@@ -59,18 +59,19 @@ def forward(self, x):
@pytest.mark.parametrize('use_bias', [True, False])
@pytest.mark.parametrize('flatten', [True, False])
@pytest.mark.parametrize('alg', fc_post_ops_list)
[email protected]("Operator square, square_root, abs, exp cannot be found in
numpy mode")
def test_fc_eltwise(data_shape, use_bias, flatten, alg):
# fc + eltwise fusion case
class FCEltwise(nn.HybridBlock):
def __init__(self, use_bias, flatten, alg, **kwargs):
super(FCEltwise, self).__init__(**kwargs)
self.fc = nn.Dense(units=64, use_bias=use_bias, flatten=flatten,
- weight_initializer=CustomNormalInit(mean=0.5,
sigma=0.1) if alg == 'square_root' else None)
+ weight_initializer=CustomNormalInit(mean=0.5,
sigma=0.1, bounded=True) if alg == 'square_root' else None)
#avoid calculating square root of
negative values
self.alg = alg
def forward(self, x):
+ if self.alg == 'square_root':
Review comment:
Is there any hidden reason besides the fact that square root can take
only 0 and positive numbers as argument? It there is not I believe the comment
is unnecessary.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
