samskalicky commented on pull request #19112: URL: https://github.com/apache/incubator-mxnet/pull/19112#issuecomment-698487976
> @samskalicky I've working on enabling MKLDNN partitioning on master and discovered that this PR broke this mobilenet_struct_v2 test. I haven't seen this PR before, so I'm sorry for late response: > I've described where problem is in the picture above. > I want to ask if you have some idea how this could be resolved. > I thought about providing additional parameter to MKLDNN fused operator, but I consider if there is some other better way? Hi @bgawrych sorry looks like I tagged the wrong people, will start tagging you for MKLDNN related issues in the future. > There seems to be a lot of hard coded magic numbers in the subgraph/mkldnn The problem that I ran into in v1.x was that the mkldnn partitioning flow was expecting the input to be duplicated, and was playing around with wiring up the the subgraph nodes. I wasnt able to decode the current flow and make it work with deduplicated inputs. Here are two of the places where the code is expecting inputs to be duplicated: https://github.com/apache/incubator-mxnet/blob/b9105784fa4d9ffc589c4d1010b3384a5420969d/src/operator/subgraph/mkldnn/mkldnn_conv.cc#L80-L83 https://github.com/apache/incubator-mxnet/blob/b9105784fa4d9ffc589c4d1010b3384a5420969d/src/operator/subgraph/mkldnn/mkldnn_conv.cc#L126-L129 Ideally, I would want to refactor the code so that there no baked in expectations. And however the subgraph is partitioned, will work with the subgraph op (ie. `SgMKLDNNConvOperator`). ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
