Skip to content

Conversation

@reyoung
Copy link
Collaborator

@reyoung reyoung commented Jun 23, 2017

The original discuess in

This commit is just a proposal, let's do such kind of summarization in
this PR.

The original discuess in

* PaddlePaddle#2548 (comment)
* PaddlePaddle#2579 (comment)

This commit is just a proposal, let's do such kind of summarization in
this PR.
@reyoung
Copy link
Collaborator Author

reyoung commented Jun 23, 2017

I just run

grep std (find . -name '*.h' -o -name '*.cpp') | sed -r 's#.*std\:\:([a-z_0-9]+)[<(\ ].*#\1#g' | grep -v 'std::' | sort | uniq -c | sort -r | head -n 10

in current PaddlePaddle repo. It returns

1116 vector
 601 string
 289 make_shared
 278 unique_ptr
 140 shared_ptr
  82 function
  76 setw
  75 dynamic_pointer_cast
  70 pair
  63 min

Maybe we should typedef:

  • map/set because the unordered_map/unordered_set is faster than default map/set. And unordered_ is too long.
  • unique_ptr/shared_ptr/make_unique(which in c++ 14 standard, but can be implemented in C++ 11 manually)/make_shared/dynamic_pointer_cast because it is heavily used and there are many smart pointer implementations.
  • vector/string/function/pair because they are heavily used.

And there are some classes in boost are very useful.

  • boost::variant and boost::any. They are in C++ 17 standard.
  • boost::non_copyable. Mark a class is non-copyable. Instead of using a macro, using this base class would not pollute the global namespace.

@reyoung reyoung changed the title Propose a typedefs header for paddle frameworks. Propose a typedefs header for paddle framework Jun 23, 2017
}

//! MakeUnique will create std::unique_ptr
template <typename T, typename... ARGS>
Copy link
Contributor

@gangliao gangliao Jun 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you did not distinguish special case, for instance, is_array<T>::value

For that case, you need to using U = typename remove_extent<T>::type;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto a_uptr = make_unique<int[]>(10);

//! Using unordered_map as Paddle's default map. It will faster than
//! std::map
template <typename Key, typename Val>
using Map = std::unordered_map<Key, Val>;
Copy link
Collaborator

@JiayiFeng JiayiFeng Jun 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about UMap or uMap?
I'm afraid most people will assume Map to be the alias of std::map at first sight.

Copy link
Collaborator

@wangkuiyi wangkuiyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using std::vector saves readers the time to grep and find out what Vector is.

Also, I don't think we will always use std::map or std::unordered_map over another one. Instead, in some curcumstances we need std::map and in some others we need std::unordered_map. It doesn't make much sense to me to stick on either of them and give the stick a name like Map.

@reyoung reyoung closed this Jun 29, 2017
@reyoung reyoung deleted the feature/typedefs_for_frameworks branch October 28, 2017 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants