HahTK commented on a change in pull request #19112:
URL: https://github.com/apache/incubator-mxnet/pull/19112#discussion_r488232578
##########
File path: src/operator/subgraph/build_subgraph.cc
##########
@@ -537,33 +537,49 @@ void FindOutputEntries(nnvm::Graph* g,
*/
void CutGraphInputs(const std::vector<nnvm::NodeEntry*> &input_entries,
std::vector<nnvm::NodeEntry> *orig_entries,
- const bool skip_var = false) {
+ std::vector<nnvm::NodeEntry> *unique_orig_entries,
+ std::vector<nnvm::NodeEntry*> *unique_input_entries,
+ const bool skip_var = false,
+ const bool dedup = false) {
Review comment:
This comment applies everywhere where we set the defaults for the dedup
flag.
It seems like this is more a problem with MKLDNN.
I understand they have done that for master branch.
Ideally, they fix it at their end for 1.8 as well.
Failing that, we should go with default = true, except when compiled for
MKLDNN and optimize_for() is called by CPU. All other paths would want to
optimize away the dupe input.
This amounts to MKLDNN patching their fix.
GIven the timelines for MXNet 1.8, this may be too tight anyway.
More importantly, the MKLDNN issues are fixed in master.
I am ok going ahead with this as is
----------------------------------------------------------------
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]
