Fix Transform API for operator#4209
Conversation
reyoung
commented
Sep 19, 2017
- Fix Some problems in paddle::platform::Transform function. #4202
| template <typename InputIter, typename OutputIter, typename UnaryOperation> | ||
| void Transform(const DeviceContext& context, InputIter first, InputIter last, | ||
| OutputIter result, UnaryOperation op) { | ||
| inline static void Transform(const DeviceContext& context, InputIter first, |
There was a problem hiding this comment.
Since we use static, the Transform will not generate symbols. cc or cu will use different Transform method.
There was a problem hiding this comment.
Thought it was the inline keyword that makes Transform not generating symbols?
Markup: https://stackoverflow.com/questions/780730/c-c-static-function-in-header-file-what-does-it-mean
There was a problem hiding this comment.
I do not think this solution is better than #4201. There are two problems with this solution:
- The
Transforminterface in .cu.o has a useless cpu code branch. - If there are many .cc and .cu are used
Transforminterface, will compile a lot of duplicate code.
There was a problem hiding this comment.
Maybe we should force inline? I think using a functor is a little bit complex.
There was a problem hiding this comment.
-
We may invoke CPU Transform in
.cufile, so that branch is not useless. Also, if we makeplaceas a template parameter, we should usePADDLE_ENFORCEto check the template place is same as thecontext.GetPlace(). So theifbranch will not be removed no matter how we implementTransform. -
In fact, the
BinaryFunctor,UnaryFunctorand theIteratorare the template parameter. Each time we invokeTransform, a new code will be generated. No matter how we implement it.
To make Transform a simple method, not a templated functor, can make users invoke it easily.
It is easier for user write Transform(ctx, input, input+size, output, Functor); than Transform<Place>()(ctx, input, input+size, output, Functor);.
| def test_check_output(self): | ||
| self.check_output() | ||
|
|
||
| def not_test_check_grad(self): |
There was a problem hiding this comment.
not_test_check_grad -> test_check_grad