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 2022/02/23 15:21:22 UTC
[GitHub] [incubator-mxnet] bartekkuncer opened a new pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
bartekkuncer opened a new pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911
## Description ##
(Brief description on what this PR is about)
## 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] anko-intel commented on a change in pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
anko-intel commented on a change in pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#discussion_r831102856
##########
File path: src/operator/nn/dnnl/dnnl_dot.cc
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file dnnl_dot.cc
+ */
+
+#if MXNET_USE_ONEDNN == 1
+
+#include "dnnl_dot-inl.h"
+
Review comment:
src/operator/nn/dnnl/dnnl_dot.cc:46: Add #include <unordered_map> for unordered_map<> [build/include_what_you_use] [4]
src/operator/nn/dnnl/dnnl_dot.cc:103: Add #include <memory> for make_shared<> [build/include_what_you_use] [4]
##########
File path: src/operator/nn/dnnl/dnnl_dot-inl.h
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file dnnl_dot-inl.h
+ */
+
+#ifndef MXNET_OPERATOR_NN_DNNL_DNNL_DOT_INL_H_
+#define MXNET_OPERATOR_NN_DNNL_DNNL_DOT_INL_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <vector>
+
+#include "dnnl_base-inl.h"
+#include "dnnl_ops-inl.h"
+#include "operator/tensor/dot-inl.h"
+
+namespace mxnet {
+namespace op {
+
+using dot_fwd_t = dnnl::matmul;
+using dot_fwd_pd_t = dnnl::matmul::primitive_desc;
+
+typedef ParamOpSign<DotParam> DotSignature;
+
+class DNNLDotFwd {
+ public:
+ static DNNLDotFwd& GetCached(const DotParam& param,
+ const std::vector<NDArray>& inputs,
+ const std::vector<NDArray>& outputs,
+ const bool isNumpy);
+
+ DNNLDotFwd(const DotParam& param,
+ const std::vector<NDArray>& inputs,
+ const std::vector<NDArray>& outputs,
+ const bool isNumpy);
+
+ void Execute(const std::vector<NDArray>& inputs,
+ const std::vector<OpReqType>& req,
+ const std::vector<NDArray>& outputs,
+ const bool isNumpy);
+
+ private:
+ std::shared_ptr<dot_fwd_t> fwd;
+ std::shared_ptr<dot_fwd_pd_t> fwd_pd;
+};
+
+template <bool isNumpy>
+void DNNLDotForward(const nnvm::NodeAttrs& attrs,
+ const OpContext& ctx,
+ const std::vector<NDArray>& inputs,
+ const std::vector<OpReqType>& req,
+ const std::vector<NDArray>& outputs) {
+ DotParam param;
+ if (isNumpy) {
+ // NumPy version of dot operator does not support transpose flags.
+ param = DotParam();
+ param.transpose_a = false;
Review comment:
should it be checked or assert only if transpose is set to false for numpy ?
##########
File path: src/operator/nn/dnnl/dnnl_dot-inl.h
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file dnnl_dot-inl.h
+ */
+
+#ifndef MXNET_OPERATOR_NN_DNNL_DNNL_DOT_INL_H_
+#define MXNET_OPERATOR_NN_DNNL_DNNL_DOT_INL_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <vector>
+
Review comment:
src/operator/nn/dnnl/dnnl_dot-inl.h:62: Add #include <memory> for shared_ptr<> [build/include_what_you_use] [4]
--
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] bartekkuncer commented on a change in pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on a change in pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#discussion_r829064997
##########
File path: src/operator/numpy/np_dot_forward.cc
##########
@@ -22,7 +22,12 @@
* \brief CPU Implementation of numpy-compatible dot
*/
-#include "./np_dot-inl.h"
+#include "np_dot-inl.h"
+#if MXNET_USE_ONEDNN == 1
+#include "operator/nn/dnnl/dnnl_ops-inl.h"
Review comment:
No need for this include here.
##########
File path: src/operator/nn/dnnl/dnnl_ops-inl.h
##########
@@ -170,6 +170,14 @@ void DNNLConcatBackward(const nnvm::NodeAttrs& attrs,
const std::vector<OpReqType>& req,
const std::vector<NDArray>& outputs);
+/* For dot */
+template <bool isNumpy>
Review comment:
No actual need for this declaration here. Maybe it is worth discussing whether we want to keep this dnnl_ops-inl.h file.
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1067754198
Jenkins CI successfully triggered : [sanity]
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1075667808
Jenkins CI successfully triggered : [unix-gpu, centos-gpu, website]
--
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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1074980917
@mxnet-bot run ci [centos-gpu, unix-gpu, unix-cpu, website]
--
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] bartekkuncer commented on a change in pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on a change in pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#discussion_r831243318
##########
File path: src/operator/nn/dnnl/dnnl_dot-inl.h
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file dnnl_dot-inl.h
+ */
+
+#ifndef MXNET_OPERATOR_NN_DNNL_DNNL_DOT_INL_H_
+#define MXNET_OPERATOR_NN_DNNL_DNNL_DOT_INL_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <vector>
+
+#include "dnnl_base-inl.h"
+#include "dnnl_ops-inl.h"
+#include "operator/tensor/dot-inl.h"
+
+namespace mxnet {
+namespace op {
+
+using dot_fwd_t = dnnl::matmul;
+using dot_fwd_pd_t = dnnl::matmul::primitive_desc;
+
+typedef ParamOpSign<DotParam> DotSignature;
+
+class DNNLDotFwd {
+ public:
+ static DNNLDotFwd& GetCached(const DotParam& param,
+ const std::vector<NDArray>& inputs,
+ const std::vector<NDArray>& outputs,
+ const bool isNumpy);
+
+ DNNLDotFwd(const DotParam& param,
+ const std::vector<NDArray>& inputs,
+ const std::vector<NDArray>& outputs,
+ const bool isNumpy);
+
+ void Execute(const std::vector<NDArray>& inputs,
+ const std::vector<OpReqType>& req,
+ const std::vector<NDArray>& outputs,
+ const bool isNumpy);
+
+ private:
+ std::shared_ptr<dot_fwd_t> fwd;
+ std::shared_ptr<dot_fwd_pd_t> fwd_pd;
+};
+
+template <bool isNumpy>
+void DNNLDotForward(const nnvm::NodeAttrs& attrs,
+ const OpContext& ctx,
+ const std::vector<NDArray>& inputs,
+ const std::vector<OpReqType>& req,
+ const std::vector<NDArray>& outputs) {
+ DotParam param;
+ if (isNumpy) {
+ // NumPy version of dot operator does not support transpose flags.
+ param = DotParam();
+ param.transpose_a = false;
Review comment:
I do not think so:
1. np.dot does not take such parameter and I am setting it here only for it to work with functions prepared for nd.dot which does take this parameter;
2. in my opinion even if we would want to have such check it should be added during parsing np.dot parameters not here;
3. API does not let np.dot execute with unknown parameters:
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1076392065
Jenkins CI successfully triggered : [unix-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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1074523336
Jenkins CI successfully triggered : [website]
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bgawrych merged pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1077693991
Jenkins CI successfully triggered : [clang]
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1048895554
Hey @bartekkuncer , 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**: [sanity, edge, miscellaneous, windows-cpu, unix-cpu, windows-gpu, clang, centos-gpu, website, unix-gpu, centos-cpu]
***
_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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1075335378
Jenkins CI successfully triggered : [unix-gpu, centos-gpu, website]
--
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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1074490782
@mxnet-bot run ci [centos-gpu, unix-gpu, unix-cpu, website]
--
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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1075407344
@mxnet-bot run ci [centos-gpu, unix-gpu, unix-cpu, website]
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1074981003
Jenkins CI successfully triggered : [centos-gpu, unix-gpu, unix-cpu, website]
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1075424360
Jenkins CI successfully triggered : [centos-gpu, unix-cpu, website, unix-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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1074523304
@mxnet-bot run ci [website]
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1067783530
Jenkins CI successfully triggered : [centos-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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1076647126
@mxnet-bot run ci [unix-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] bartekkuncer commented on a change in pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on a change in pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#discussion_r829066042
##########
File path: src/operator/nn/dnnl/dnnl_ops-inl.h
##########
@@ -170,6 +170,14 @@ void DNNLConcatBackward(const nnvm::NodeAttrs& attrs,
const std::vector<OpReqType>& req,
const std::vector<NDArray>& outputs);
+/* For dot */
+template <bool isNumpy>
Review comment:
No actual need for this declaration here. Maybe worth discussing whether we want to keep this dnnl_ops-inl.h file.
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1062817523
Jenkins CI successfully triggered : [miscellaneous, unix-cpu, website, windows-cpu, unix-gpu, windows-gpu, centos-gpu, centos-cpu, sanity, edge, clang]
--
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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1076158245
@mxnet-bot run ci [centos-cpu, unix-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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1076647221
Jenkins CI successfully triggered : [unix-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] bartekkuncer commented on a change in pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on a change in pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#discussion_r829066042
##########
File path: src/operator/nn/dnnl/dnnl_ops-inl.h
##########
@@ -170,6 +170,14 @@ void DNNLConcatBackward(const nnvm::NodeAttrs& attrs,
const std::vector<OpReqType>& req,
const std::vector<NDArray>& outputs);
+/* For dot */
+template <bool isNumpy>
Review comment:
No actual need for this declaration here. Maybe worth discussing whether we want to keep this dnnl_ops-inl.h file.
##########
File path: src/operator/numpy/np_dot_forward.cc
##########
@@ -22,7 +22,12 @@
* \brief CPU Implementation of numpy-compatible dot
*/
-#include "./np_dot-inl.h"
+#include "np_dot-inl.h"
+#if MXNET_USE_ONEDNN == 1
+#include "operator/nn/dnnl/dnnl_ops-inl.h"
Review comment:
No need for this include here.
##########
File path: src/operator/nn/dnnl/dnnl_ops-inl.h
##########
@@ -170,6 +170,14 @@ void DNNLConcatBackward(const nnvm::NodeAttrs& attrs,
const std::vector<OpReqType>& req,
const std::vector<NDArray>& outputs);
+/* For dot */
+template <bool isNumpy>
Review comment:
No actual need for this declaration here. Maybe it is worth discussing whether we want to keep this dnnl_ops-inl.h file.
--
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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1067783471
@mxnet-bot run ci [centos-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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1074324052
Jenkins CI successfully triggered : [unix-gpu, website, 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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1075667756
@mxnet-bot run ci [centos-gpu, unix-gpu, website]
--
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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1076391973
@mxnet-bot run ci [unix-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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1075244150
@mxnet-bot run ci [centos-gpu, unix-gpu, unix-cpu, website]
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1075244258
Jenkins CI successfully triggered : [website, centos-gpu, unix-cpu, unix-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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1075335309
@mxnet-bot run ci [centos-gpu, unix-gpu, website]
--
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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1062812767
@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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1062817422
@mxnet-bot run ci [all]
--
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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1067754147
@mxnet-bot run ci [sanity]
--
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] bartekkuncer commented on a change in pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on a change in pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#discussion_r826863212
##########
File path: src/operator/nn/dnnl/dnnl_dot.cc
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file dnnl_dot.cc
+ */
+
+#if MXNET_USE_ONEDNN == 1
+
+#include "dnnl_dot-inl.h"
+
+namespace mxnet {
+namespace op {
+
+bool SupportDNNLDot(const std::vector<NDArray>& inputs, const NDArray& output) {
+#if MXNET_USE_BLAS_MKL == 1
+ return false;
+#endif
+ return inputs[DotIn::lhs].shape().Size() > 1 && inputs[DotIn::rhs].shape().Size() > 1 &&
+ inputs[DotIn::lhs].shape().ndim() > 0 && inputs[DotIn::rhs].shape().ndim() > 0 &&
+ output.shape().Size() != 0 && output.shape().ndim() > 0 && output.shape().ndim() <= 12 &&
+ (inputs[DotIn::lhs].dtype() == mshadow::kFloat32 ||
+ inputs[DotIn::lhs].dtype() == mshadow::kBfloat16);
+}
+
+DNNLDotFwd& DNNLDotFwd::GetCached(const DotParam& param,
+ const std::vector<NDArray>& inputs,
+ const std::vector<NDArray>& outputs,
+ const bool isNumpy) {
+ using dot_fwd_map = std::unordered_map<DotSignature, DNNLDotFwd, OpHash>;
+#if DMLC_CXX11_THREAD_LOCAL
+ static thread_local dot_fwd_map fwds;
+#else
+ static MX_THREAD_LOCAL dot_fwd_map fwds;
+#endif
+
+ DotSignature key(param);
+ key.AddSign(inputs[DotIn::lhs]);
+ key.AddSign(inputs[DotIn::rhs]);
+ key.AddSign(outputs[DotOut::out]);
+ key.AddSign(static_cast<int>(isNumpy));
+
+ auto it = fwds.find(key);
+ if (it == fwds.end()) {
+ const DNNLDotFwd fwd(param, inputs, outputs, isNumpy);
+ it = AddToCache(&fwds, key, fwd);
+ }
+ return it->second;
+}
+
+auto GetMemoryDesc(const NDArray& tensor, int firstDim, int secondDim, const bool transpose) {
+ return dnnl::memory::desc(
+ dnnl::memory::dims{firstDim, secondDim},
+ get_dnnl_type(tensor.dtype()),
+ transpose ? dnnl::memory::format_tag::ba : dnnl::memory::format_tag::ab);
+}
+
+DNNLDotFwd::DNNLDotFwd(const DotParam& param,
+ const std::vector<NDArray>& inputs,
+ const std::vector<NDArray>& outputs,
+ const bool isNumpy) {
+ auto shapeLhs = inputs[DotIn::lhs].shape(), shapeRhs = inputs[DotIn::rhs].shape();
+ auto ndimLhs = shapeLhs.ndim(), ndimRhs = shapeRhs.ndim();
+ dnnl::memory::desc lhs_md, rhs_md, out_md;
+ // NumPy expects more than 2 dimensional rhs tensor as Ax...xKxN which is different than NDArray's
+ // KxAx...xN format. For NumPy shape in rhs memory descriptor is going to be Kx(A*...*N),
+ // similarly to NDArray, but for it to match the actual data there will be an additional reorder
Review comment:
```suggestion
// similarly to NDArray, but for it to match the actual data there is an additional reorder
```
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1076158366
Jenkins CI successfully triggered : [unix-gpu, centos-cpu, 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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1077693880
@mxnet-bot run ci [clang]
--
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 #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1074490856
Jenkins CI successfully triggered : [unix-gpu, website, 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] bartekkuncer commented on pull request #20911: [operator] Integrate oneDNN matmul primitive to mxnet dot operator
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on pull request #20911:
URL: https://github.com/apache/incubator-mxnet/pull/20911#issuecomment-1074323945
@mxnet-bot run ci [centos-gpu, unix-gpu, website]
--
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