[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #14940: Fix warnings
apeforest commented on a change in pull request #14940: Fix warnings URL: https://github.com/apache/incubator-mxnet/pull/14940#discussion_r314596452 ## File path: src/operator/nn/mkldnn/mkldnn_fully_connected.cc ## @@ -26,6 +26,7 @@ #if MXNET_USE_MKLDNN == 1 #include "mkldnn_fully_connected-inl.h" +#include Review comment: @larroy Please don't be so defensive. I simply didn't understand why if a missing header file in the code would cause warning rather than just failing to compile. It would be great if you could answer my question and thanks @ChaiBapchya for pointing me to the location. But again, why it would pass compilation earlier if that needed STL header was missing? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #14940: Fix warnings
apeforest commented on a change in pull request #14940: Fix warnings URL: https://github.com/apache/incubator-mxnet/pull/14940#discussion_r314590377 ## File path: include/mxnet/executor.h ## @@ -28,6 +28,8 @@ #include #include +#include Review comment: where is this needed and which warning did this fix? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #14940: Fix warnings
apeforest commented on a change in pull request #14940: Fix warnings URL: https://github.com/apache/incubator-mxnet/pull/14940#discussion_r314589743 ## File path: src/operator/nn/mkldnn/mkldnn_concat.cc ## @@ -25,6 +25,7 @@ #if MXNET_USE_MKLDNN == 1 #include "mkldnn_concat-inl.h" +#include Review comment: where is this needed. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #14940: Fix warnings
apeforest commented on a change in pull request #14940: Fix warnings URL: https://github.com/apache/incubator-mxnet/pull/14940#discussion_r314589730 ## File path: src/operator/nn/mkldnn/mkldnn_fully_connected.cc ## @@ -26,6 +26,7 @@ #if MXNET_USE_MKLDNN == 1 #include "mkldnn_fully_connected-inl.h" +#include Review comment: where is this needed? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #14940: Fix warnings
apeforest commented on a change in pull request #14940: Fix warnings URL: https://github.com/apache/incubator-mxnet/pull/14940#discussion_r314589762 ## File path: src/operator/nn/cudnn/cudnn_algoreg-inl.h ## @@ -30,6 +30,7 @@ #include #include #include +#include Review comment: where is this needed? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #14940: Fix warnings
apeforest commented on a change in pull request #14940: Fix warnings
URL: https://github.com/apache/incubator-mxnet/pull/14940#discussion_r314589619
##
File path: src/common/exec_utils.h
##
@@ -49,85 +49,19 @@ namespace common {
indices are not recorded
* \return true if any source NDArray need to cast storage
*/
-inline bool SetupDefaultBlobsIn(const std::vector& src,
Review comment:
What warnings did this cause?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #14940: Fix warnings
apeforest commented on a change in pull request #14940: Fix warnings URL: https://github.com/apache/incubator-mxnet/pull/14940#discussion_r314589474 ## File path: src/common/exec_utils.cc ## @@ -0,0 +1,519 @@ +/* Review comment: I failed to understand what warnings did this fix? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #14940: Fix warnings
apeforest commented on a change in pull request #14940: Fix warnings
URL: https://github.com/apache/incubator-mxnet/pull/14940#discussion_r314589551
##
File path: src/common/exec_utils.cc
##
@@ -0,0 +1,526 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#include "exec_utils.h"
+
+namespace mxnet {
+namespace common {
+
+
+bool SetupDefaultBlobsIn(const std::vector& src,
+const std::vector *bufs,
+std::vector *blobs,
+std::vector *temp_src,
+std::vector *temp_dst,
+std::unordered_map
*idx_map) {
+ bool require_cast = false;
+ for (size_t i = 0; i < src.size(); i++) {
+auto& nd = src[i];
+bool is_default = nd.storage_type() == kDefaultStorage;
+#if MXNET_USE_MKLDNN == 1
+// We have to make sure it's default storage and default layout.
+is_default = nd.IsDefaultData();
+#endif
+if (!is_default) {
+ (*idx_map)[i] = temp_dst->size();
+ NDArray temp = bufs != nullptr ? bufs->at(i) : NDArray(nd.shape(),
nd.ctx(),
+ true, nd.dtype());
+#if MXNET_USE_MKLDNN == 1
+ CHECK(temp.IsDefaultData());
+#endif
+ temp_src->emplace_back(nd);
+ temp_dst->emplace_back(temp);
+ blobs->emplace_back(temp.data());
+ require_cast = true;
+} else {
+ blobs->push_back(nd.data());
+}
+ }
+ return require_cast;
+}
+
+bool SetupDefaultBlobsOut(const std::vector& src,
+ const std::vector *bufs,
+ std::vector *req,
+ std::vector *blobs,
+ std::vector *temp_src,
+ std::vector *temp_dst) {
+ bool require_cast = false;
+ for (size_t i = 0; i < src.size(); i++) {
+auto& nd = src[i];
+bool is_default = nd.storage_type() == kDefaultStorage;
+#if MXNET_USE_MKLDNN == 1
+if (req->at(i) == kWriteInplace && nd.IsMKLDNNData())
+ // If it's write inplace and the output array doesn't use the default
+ // layout, we'll generate a temporary output array below, which means
+ // the input array and the output array are no longer the same array.
+ // we should change the request type.
+ req->at(i) = kWriteTo;
+// We have to make sure it's default storage and default layout.
+is_default = nd.IsDefaultData();
+#endif
+if (!is_default) {
+#if MXNET_USE_MKLDNN == 1
+ NDArray temp;
+ if (bufs != nullptr) {
+temp = bufs->at(i);
+ } else if (kAddTo == req->at(i) && nd.IsMKLDNNData()) {
+temp = nd.Reorder2Default();
+ } else if (kAddTo == req->at(i)) {
+temp = nd;
+ } else {
+temp = NDArray(nd.shape(), nd.ctx(), true, nd.dtype());
+ }
+ CHECK(temp.IsDefaultData());
+#else
+ NDArray temp = bufs != nullptr ? bufs->at(i) : NDArray(nd.shape(),
nd.ctx(),
+ true, nd.dtype());
+#endif
+ temp_src->emplace_back(nd);
+ temp_dst->emplace_back(temp);
+ blobs->emplace_back(temp.data());
+ require_cast = true;
+} else {
+ blobs->push_back(nd.data());
+}
+ }
+ return require_cast;
+}
+
+
+void SetupDefaultBlobsInOut(const std::vector ,
+ const std::vector ,
+ const std::vector *in_bufs,
+ const std::vector *out_bufs,
+ std::vector *req,
+ std::vector *input_blobs,
+ std::vector *output_blobs,
+ std::vector *pre_temp_src,
+ std::vector *pre_temp_dst,
+ std::vector *post_temp_src,
+ std::vector *post_temp_dst,
+ std::unordered_map
*in_temp_idx_map,
+ const std::vector _idx) {
+ // populate input blobs
+ SetupDefaultBlobsIn(ndinputs, in_bufs, input_blobs, pre_temp_src,
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #14940: Fix warnings
apeforest commented on a change in pull request #14940: Fix warnings
URL: https://github.com/apache/incubator-mxnet/pull/14940#discussion_r314589408
##
File path: src/common/exec_utils.cc
##
@@ -0,0 +1,526 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#include "exec_utils.h"
+
+namespace mxnet {
+namespace common {
+
+
+bool SetupDefaultBlobsIn(const std::vector& src,
+const std::vector *bufs,
+std::vector *blobs,
+std::vector *temp_src,
+std::vector *temp_dst,
+std::unordered_map
*idx_map) {
+ bool require_cast = false;
+ for (size_t i = 0; i < src.size(); i++) {
+auto& nd = src[i];
+bool is_default = nd.storage_type() == kDefaultStorage;
+#if MXNET_USE_MKLDNN == 1
+// We have to make sure it's default storage and default layout.
+is_default = nd.IsDefaultData();
+#endif
+if (!is_default) {
+ (*idx_map)[i] = temp_dst->size();
+ NDArray temp = bufs != nullptr ? bufs->at(i) : NDArray(nd.shape(),
nd.ctx(),
+ true, nd.dtype());
+#if MXNET_USE_MKLDNN == 1
+ CHECK(temp.IsDefaultData());
+#endif
+ temp_src->emplace_back(nd);
+ temp_dst->emplace_back(temp);
+ blobs->emplace_back(temp.data());
+ require_cast = true;
+} else {
+ blobs->push_back(nd.data());
+}
+ }
+ return require_cast;
+}
+
+bool SetupDefaultBlobsOut(const std::vector& src,
+ const std::vector *bufs,
+ std::vector *req,
+ std::vector *blobs,
+ std::vector *temp_src,
+ std::vector *temp_dst) {
+ bool require_cast = false;
+ for (size_t i = 0; i < src.size(); i++) {
+auto& nd = src[i];
+bool is_default = nd.storage_type() == kDefaultStorage;
+#if MXNET_USE_MKLDNN == 1
+if (req->at(i) == kWriteInplace && nd.IsMKLDNNData())
+ // If it's write inplace and the output array doesn't use the default
+ // layout, we'll generate a temporary output array below, which means
+ // the input array and the output array are no longer the same array.
+ // we should change the request type.
+ req->at(i) = kWriteTo;
+// We have to make sure it's default storage and default layout.
+is_default = nd.IsDefaultData();
+#endif
+if (!is_default) {
+#if MXNET_USE_MKLDNN == 1
+ NDArray temp;
+ if (bufs != nullptr) {
+temp = bufs->at(i);
+ } else if (kAddTo == req->at(i) && nd.IsMKLDNNData()) {
+temp = nd.Reorder2Default();
+ } else if (kAddTo == req->at(i)) {
+temp = nd;
+ } else {
+temp = NDArray(nd.shape(), nd.ctx(), true, nd.dtype());
+ }
+ CHECK(temp.IsDefaultData());
+#else
+ NDArray temp = bufs != nullptr ? bufs->at(i) : NDArray(nd.shape(),
nd.ctx(),
+ true, nd.dtype());
+#endif
+ temp_src->emplace_back(nd);
+ temp_dst->emplace_back(temp);
+ blobs->emplace_back(temp.data());
+ require_cast = true;
+} else {
+ blobs->push_back(nd.data());
+}
+ }
+ return require_cast;
+}
+
+
+void SetupDefaultBlobsInOut(const std::vector ,
+ const std::vector ,
+ const std::vector *in_bufs,
+ const std::vector *out_bufs,
+ std::vector *req,
+ std::vector *input_blobs,
+ std::vector *output_blobs,
+ std::vector *pre_temp_src,
+ std::vector *pre_temp_dst,
+ std::vector *post_temp_src,
+ std::vector *post_temp_dst,
+ std::unordered_map
*in_temp_idx_map,
+ const std::vector _idx) {
+ // populate input blobs
+ SetupDefaultBlobsIn(ndinputs, in_bufs, input_blobs, pre_temp_src,
