New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New FunctionTest #1132

Merged

hedaoyuan merged 12 commits into PaddlePaddle:develop from hedaoyuan:FunctionTest

Jan 17, 2017

Contributor

hedaoyuan commented Jan 12, 2017 •

edited

Loading

/**
 * \brief A class for comparing CPU and GPU implementations of Function.
 *
 *
 * Use case:
 *  // Initializes a test object, the corresponding cpu and gpu Function
 *  // are constructed according to FunctionName and FuncConfig.
 *  FunctionCompare test(FunctionName, FuncConfig);
 *  // Prepare inputs and outputs arguments.
 *  // Here the input and output can not contain real data,
 *  // only contains the argument type and shape.
 *  test.addInputs(input1);
 *  test.addInputs(input2);
 *  test.addOutputs(output1);
 *  test.addOutputs(output2);
 *  // Run.
 *  // Will according to the type and shape of arguments(inputs_/outputs_),
 *  // automatic initialization cpu and gpu function required arguments
 *  // (cpuInputs_/cpuOutputs_/gpuInputs_/gpuOutputs_).
 *  // Call the CPU and GPU Function calculation results.
 *  // Compares CPU and GPU calculation results for consistency.
 *  test.run();
 */
class FunctionCompare


          New FunctionTest

2629d43

hedaoyuan mentioned this pull request

Paddle Function #892

Closed

hedaoyuan added 4 commits

January 12, 2017 18:03


          move a test case from BufferArgTest.cpp to FunctionTest.cpp

fdf194a


          Add a CheckBufferArg.

bff19f5

It is used to check the consistency between the BufferArg type argument
received by Function and the original type argument.


          Change BufferArgPtr to BufferArg*

2a20fdc


          Add some constructors for generating object that only contains shape …

039c0bf

…(do not contains data).

hedaoyuan requested a review from tianbingsz

January 13, 2017 07:07

hedaoyuan added 6 commits

January 13, 2017 17:46


          Implement the FunctionTest


          Fix CrossMapNormal Test

b791dcd


          Merge branch 'develop' of https://github.com/baidu/Paddle into Functi…

97d6bc6

…onTest


          Merge branch 'develop' of https://github.com/baidu/Paddle into Functi…

283bee2

…onTest


          Fix merge error

1e23317


          Fix CrossMapNormalTest

f8c9c88

tianbingsz approved these changes

View reviewed changes

Contributor

tianbingsz left a comment

Looks very good to me. Approve it to make you move fast. Only minor comments need to be addressed.

paddle/function/FunctionTest.h

+                      std::make_shared<BufferArg>(cpuMemory_.back()->getBuf(),
+                                                  output.valueType(),
+                                                  output.shape(),
+                                                  ASSIGN_TO));

Contributor

tianbingsz Jan 17, 2017

should we use argType here? pass from void addOutputs(const BufferArg& output, argType) ?

Contributor Author

hedaoyuan Jan 17, 2017

嗯，这里需要再考虑一下，目前有的Function实现上暂时只支持ADD_TO。我本来想的是是否所有Function都需要支持ASSIGN_TO。

paddle/function/FunctionTest.h

+                      std::make_shared<BufferArg>(gpuMemory_.back()->getBuf(),
+                                                  output.valueType(),
+                                                  output.shape(),
+                                                  ASSIGN_TO));

Contributor

tianbingsz Jan 17, 2017

same as line 82.

Contributor Author

hedaoyuan Jan 17, 2017

同上。

paddle/function/FunctionTest.h


		// TODO: need be implemented.
		}

Contributor

tianbingsz Jan 17, 2017

TODO, we may also need void addInputs(const SparseMatrixArg& input), I think I can do it later.

Contributor Author

hedaoyuan Jan 17, 2017

是的。

paddle/function/FunctionTest.h

+                void initArg(SequenceIdArg& arg, size_t batchSize) {
+                  size_t numSeqs = arg.numSeqs();
+                  int* buf = (int*)arg.data();

Contributor

tianbingsz Jan 17, 2017

can we use reinterpret_cast<int*>(arg.data())?

Contributor Author

hedaoyuan Jan 17, 2017

done。

paddle/function/FunctionTest.h

+                    autotest::TensorCheckErr(cpuVector, gpuVector);
+                  }
+                }

Contributor

tianbingsz Jan 17, 2017

TODO, do we need to compare the output of two sparse matrice or two sequences? If needed, we can do it later.

Contributor Author

hedaoyuan Jan 17, 2017

嗯，是需要的。

paddle/function/FunctionTest.h

+                                         std::vector<BufferArgPtr>& outputs) {
+                    BufferArgs inArgs;
+                    BufferArgs outArgs;
+                    for (auto arg : inputs) {

Contributor

tianbingsz Jan 17, 2017

const auto arg?

Contributor Author

hedaoyuan Jan 17, 2017

同上。

paddle/function/FunctionTest.cpp

+              }
+              void testBufferArgs(const BufferArgs& inputs, const CheckBufferArg& check) {
+                check(inputs[0]);

Contributor

tianbingsz Jan 17, 2017

should we add CHECK_GE(inputs.size(), 1)?

Contributor Author

hedaoyuan Jan 17, 2017

done。

paddle/function/FunctionTest.cpp

+                argments.addArg(*matrix);
+                std::vector<CheckBufferArg> checkFunc;
+                checkFunc.push_back(check);
+                testBufferArgs(argments, checkFunc);

Contributor

tianbingsz Jan 17, 2017

Since we only have one buffer arg and check function, can we directly write: check(argments)? Just curious :)

Contributor Author

hedaoyuan Jan 17, 2017

testBufferArgs(argments, checkFunc);这么写目的就是构造Function的调用过程；直接写成check(argments)的话相当于直接在TEST里面check。

paddle/function/Function.h

+                ~BufferArgs() {
+                  for (auto arg : _args_) {
+                    delete arg;

Contributor

tianbingsz Jan 17, 2017

Curious, if args delete its arg, what will happen to the corresponding element in args_(std::vector<BufferArg*>)? Do we really need to worry about it?

Contributor Author

hedaoyuan Jan 17, 2017

这里只是防止内存泄漏，~BufferArgs析构后，再使用BufferArgs::args_已经是非法了。

paddle/function/BufferArg.h

+                SequenceIdArg(const TensorShape& shape, ArgType argType = UNSPECIFIED)
+                    : BufferArg(VALUE_TYPE_INT32, shape, argType) {
+                  CHECK_EQ(shape_.ndims(), (size_t)1);
+                  numSeqs_ = shape_[0] - 1;

Contributor

tianbingsz Jan 17, 2017

Do we need to CHECK(shape_[0] > 1) or CHECK(shape_[0] >= 1) ?

Contributor Author

hedaoyuan Jan 17, 2017

Done。
Add CHECK_GT(shape_[0], 1);


          follow comments

1c5a7c4

hedaoyuan merged commit 297f06c into PaddlePaddle:develop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet