Merged
Conversation
Superjomn
requested review from
jacquesqiao,
lcy-seso,
luotao1,
qingqing01 and
zchen0211
jacquesqiao
reviewed
Sep 30, 2017
paddle/framework/tensor_array.cc
Outdated
|
|
||
| namespace detail { | ||
|
|
||
| void ta_check_index(size_t index, size_t MAX_SIZE) { |
Member
There was a problem hiding this comment.
may be just check_index? ta is a litter confusing
jacquesqiao
reviewed
Sep 30, 2017
paddle/framework/tensor_array.cc
Outdated
|
|
||
| namespace detail { | ||
|
|
||
| void ta_check_index(size_t index, size_t MAX_SIZE) { |
Member
There was a problem hiding this comment.
这个大写的 MAX_SIZE 和定义的那个const看起来有点容易混淆
jacquesqiao
reviewed
Sep 30, 2017
| return detail::PackDynamicBatch(values_, meta, lod, level); | ||
| } | ||
|
|
||
| std::vector<DySeqMeta> TensorArray::Unpack(const LoDTensor& source, int level, |
Member
There was a problem hiding this comment.
这个函数里面，似乎没有用到 length_desend 这个参数？
jacquesqiao
reviewed
Sep 30, 2017
| * Split LoDTensor in some `level` and write the generated batches to | ||
| * `values`, if set `desend`, will sort by length in descending order. | ||
| */ | ||
| std::vector<DySeqMeta> Unpack(const LoDTensor &source, int level, |
Member
There was a problem hiding this comment.
what if length_desend is set to false
jacquesqiao
reviewed
Sep 30, 2017
| void Unstack(const LoDTensor &source, bool data_shared) const; | ||
|
|
||
| private: | ||
| mutable std::vector<LoDTensor> values_; |
Contributor
Author
There was a problem hiding this comment.
yes, it will be changed in Read(), expand dynamically.
jacquesqiao
reviewed
Sep 30, 2017
| @@ -0,0 +1,114 @@ | |||
| #include "paddle/framework/tensor_array.h" | |||
jacquesqiao
reviewed
Sep 30, 2017
| } | ||
|
|
||
| TensorArray ta; | ||
| const int batch_size{16}; |
Member
There was a problem hiding this comment.
jacquesqiao
reviewed
Sep 30, 2017
| LoDTensor packed = ta.Pack(0, meta, lod); | ||
| } | ||
|
|
||
| TEST_F(TensorArrayTester, size) { ASSERT_EQ(ta.size(), (size_t)batch_size); } |
Member
There was a problem hiding this comment.
use cpp style cast instead of c style cast
https://google.github.io/styleguide/cppguide.html#Casting
jacquesqiao
reviewed
Sep 30, 2017
| using value_type = float; | ||
|
|
||
| // max number of values allowed to store. | ||
| const size_t MAX_SIZE{100000}; |
Member
There was a problem hiding this comment.
jacquesqiao
approved these changes
Sep 30, 2017
Member
jacquesqiao
left a comment
jacquesqiao
left a comment
There was a problem hiding this comment.
LGTM! except for some const value name style
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.