Add Majel's Place concept to Paddle#2091
Conversation
Dockerfile
Outdated
There was a problem hiding this comment.
@wangkuiyi It's better to replace boost using some light-weight implementation https://github.com/mapbox/variant since it's too heavy.
There was a problem hiding this comment.
https://github.com/PaddlePaddle/Paddle/blob/develop/cmake/external/glog.cmake#L29 It's a good way to integrate external project via CMake.
There was a problem hiding this comment.
About boost
I listed all boost packages that Majel depends on:
$ for i in $(du -a | grep '\.h$' | cut -f 2); do \
grep 'boost::' $i; \
done | \
grep -o 'boost::[a-zA-Z_]*' | sort | uniq
boost::apply_visitor
boost::bad_get
boost::get
boost::python
boost::shared_ptr
boost::static_visitor
boost::variantI am afraid that we cannot replace all of them using some lighter weighted versions?
There was a problem hiding this comment.
I understand and agree that we should use CMake and its external projects. I didn't do it in this PR because I want to figure out what is the command line we need to build Majel.
| @@ -0,0 +1,10 @@ | |||
| CCFLAGS = -std=c++11 -I/work/paddle | |||
There was a problem hiding this comment.
After this PR is merged, I will add a CMakeLists.txt.
| // needed for variant equality comparison | ||
| inline bool operator==(const GpuPlace& o) const { return device == o.device; } | ||
|
|
||
| inline bool operator!=(const GpuPlace& o) const { return !(*this == o); } |
There was a problem hiding this comment.
Why the above "==" operation check equal as device == o.device, but here checks if it's the same instance?
There was a problem hiding this comment.
You are right! Is Greg nearby and can you ask him about the problem?
There was a problem hiding this comment.
My bad, I thought *this is this. In this case, it's taking the neg of result from == operator.
There was a problem hiding this comment.
@wangkuiyi @helinwang I think it's correct. Because *this == o will trigger inline bool operator==(const GpuPlace& o) So it's still compare device id.
Fixes #2090
Note: Please review and merge #2040 before this. In order to build the ported Majel code, I am using the development Docker image.