Return size_t from shape::size().#367
Conversation
|
@apeforest Please help review and merge. Thanks. |
|
@szha We have an offline talk with @apeforest and we agreed with the change to fix the casting issue in MXNet. Please help take a review and merge this PR. Happy New Year 👍 |
There was a problem hiding this comment.
@ZhennanQin Could you please link the PR (ready for review) in MXNet that requires this change?
|
It's apache/mxnet#13697 If we cast manually to size_t it compiles. For reference: apache/mxnet#13755 |
|
@szha Please help to review and merge this. Then I will upgrade mshadow in apache/mxnet#13729. |
| /*! \return number of valid elements */ | ||
| MSHADOW_XINLINE index_t Size(void) const { | ||
| MSHADOW_XINLINE size_t Size(void) const { | ||
| index_t size = this->shape_[0]; |
There was a problem hiding this comment.
Maybe we want to cast here to size_t explicitly?
|
@ZhennanQin could you address the comments Thanks! |
|
@stu1130 The comment request is beyond my initial scope, and I don't have enough knowledge to address other API changes. As it's not a block issue for me anymore, let @apeforest to continue this work. I can abandon this PR. |
5 participants
This issue blocks MXNet PR: apache/mxnet#13697. apache/mxnet#13729 is the corresponding fix for MXNet. After this fix, that PR can pass. Ref to apache/mxnet#13755.