-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
pp_A.reserve(batch_count); | ||
pp_B.reserve(batch_count); | ||
pp_C.reserve(batch_count); | ||
std::vector<const double*> pp_A(batch_count, nullptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the fix!
It will be faster to use std::vector<const double*> pp_A(batch_count);
: )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this because it's precisely the same logic used by the earlier commit.
That being said, I believe the STL guarantees that non-class types will be zero initialized when using the constructor form you propose, so I would expect the performance of the two forms to be identical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you!
Hi @aws-taylor , I could not retrigger the CI. Thank you! --edit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the fix! Sorry I missed the double precision part in previous commit.
The PR has been merged. Thank you! |
* Additional fix for vector access. See apache@9634786 for the original. * CI * ci * ci * retrigger CI * ci Co-authored-by: JackieWu <[email protected]>
* Additional fix for vector access. See apache@9634786 for the original. * CI * ci * ci * retrigger CI * ci Co-authored-by: JackieWu <[email protected]>
See 9634786 for the original.
Description
Fixes an assertion thrown by operator[] of std::vector when MxNet is compiled with certain STL hardening flags, especially since GCC 8. This pull request is to fix a small block that was missed in the original PR.