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 2021/07/26 20:47:23 UTC
[GitHub] [incubator-mxnet] Kh4L opened a new pull request #20470: Fix an array iterator in nnvm_to_onnx
Kh4L opened a new pull request #20470:
URL: https://github.com/apache/incubator-mxnet/pull/20470
## Description ##
Fix the issue described in https://github.com/apache/incubator-mxnet/issues/20376
--
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] Kh4L commented on a change in pull request #20470: Fix an array iterator in nnvm_to_onnx
Posted by GitBox <gi...@apache.org>.
Kh4L commented on a change in pull request #20470:
URL: https://github.com/apache/incubator-mxnet/pull/20470#discussion_r691045830
##########
File path: src/operator/subgraph/tensorrt/nnvm_to_onnx.cc
##########
@@ -776,7 +776,7 @@ void ConvertConstant(
std::shared_ptr<uint16_t[]> shared_data_ptr(new uint16_t[size]);
uint16_t* const data_ptr = shared_data_ptr.get();
nd.SyncCopyToCPU(static_cast<void*>(data_ptr), size);
- for (size_t blob_idx = 0; blob_idx < size; ++blob_idx) {
+ for (size_t blob_idx = 0; blob_idx < size / 2; ++blob_idx) {
Review comment:
Right, good catch!
--
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 #20470: Fix an array iterator in nnvm_to_onnx
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20470:
URL: https://github.com/apache/incubator-mxnet/pull/20470#issuecomment-887414476
Jenkins CI successfully triggered : [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] Kh4L commented on a change in pull request #20470: Fix an array iterator in nnvm_to_onnx
Posted by GitBox <gi...@apache.org>.
Kh4L commented on a change in pull request #20470:
URL: https://github.com/apache/incubator-mxnet/pull/20470#discussion_r691045830
##########
File path: src/operator/subgraph/tensorrt/nnvm_to_onnx.cc
##########
@@ -776,7 +776,7 @@ void ConvertConstant(
std::shared_ptr<uint16_t[]> shared_data_ptr(new uint16_t[size]);
uint16_t* const data_ptr = shared_data_ptr.get();
nd.SyncCopyToCPU(static_cast<void*>(data_ptr), size);
- for (size_t blob_idx = 0; blob_idx < size; ++blob_idx) {
+ for (size_t blob_idx = 0; blob_idx < size / 2; ++blob_idx) {
Review comment:
Right, good catch!
--
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 #20470: Fix an array iterator in nnvm_to_onnx
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20470:
URL: https://github.com/apache/incubator-mxnet/pull/20470#issuecomment-914318630
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] Kh4L commented on pull request #20470: Fix an array iterator in nnvm_to_onnx
Posted by GitBox <gi...@apache.org>.
Kh4L commented on pull request #20470:
URL: https://github.com/apache/incubator-mxnet/pull/20470#issuecomment-887414429
@mxnet-bot run ci [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] ptrendx commented on pull request #20470: Fix an array iterator in nnvm_to_onnx
Posted by GitBox <gi...@apache.org>.
ptrendx commented on pull request #20470:
URL: https://github.com/apache/incubator-mxnet/pull/20470#issuecomment-917032352
Hmm, I don't think that the fix for the odd size is right - you still can read beyond the original size. I would frankly just allocate the shared_data_ptr as uint32_t with `(size + 1) / 2` elements and then set the last element to 0 if the size is odd (just to not include garbage in your actual output). Then do the memcopy. That way you avoid 2 problems:
- currently your shared_data_ptr could be aligned to just 2 bytes, whereas to read it as uint32_t you need to have 4 byte alignment
- if the size is odd you still read beyond the end of array
--
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] Kh4L commented on pull request #20470: Fix an array iterator in nnvm_to_onnx
Posted by GitBox <gi...@apache.org>.
Kh4L commented on pull request #20470:
URL: https://github.com/apache/incubator-mxnet/pull/20470#issuecomment-914318551
@mxnet-bot run ci [unix-gpu, linkcheck]
--
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] ptrendx commented on a change in pull request #20470: Fix an array iterator in nnvm_to_onnx
Posted by GitBox <gi...@apache.org>.
ptrendx commented on a change in pull request #20470:
URL: https://github.com/apache/incubator-mxnet/pull/20470#discussion_r685381910
##########
File path: src/operator/subgraph/tensorrt/nnvm_to_onnx.cc
##########
@@ -776,7 +776,7 @@ void ConvertConstant(
std::shared_ptr<uint16_t[]> shared_data_ptr(new uint16_t[size]);
uint16_t* const data_ptr = shared_data_ptr.get();
nd.SyncCopyToCPU(static_cast<void*>(data_ptr), size);
- for (size_t blob_idx = 0; blob_idx < size; ++blob_idx) {
+ for (size_t blob_idx = 0; blob_idx < size / 2; ++blob_idx) {
Review comment:
What if size is odd?
--
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 #20470: Fix an array iterator in nnvm_to_onnx
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20470:
URL: https://github.com/apache/incubator-mxnet/pull/20470#issuecomment-887015208
Hey @Kh4L , 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**: [website, edge, miscellaneous, centos-gpu, windows-gpu, unix-cpu, clang, centos-cpu, sanity, unix-gpu, windows-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