Skip to content

Commit

Permalink
code refine
Browse files Browse the repository at this point in the history
  • Loading branch information
chengduoZH committed Apr 12, 2018
1 parent ae5923e commit e26c6d7
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 100 deletions.
10 changes: 5 additions & 5 deletions paddle/fluid/framework/details/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
cc_library(var_handle SRCS var_handle.cc DEPS place)
cc_library(op_handle_base SRCS op_handle_base.cc DEPS var_handle device_context)
cc_library(op_handle_base SRCS op_handle_base.cc DEPS var_handle device_context lod_tensor)
cc_library(scale_loss_grad_op_handle SRCS scale_loss_grad_op_handle.cc DEPS op_handle_base scope lod_tensor ddim memory)
cc_library(fetch_op_handle SRCS fetch_op_handle.cc DEPS op_handle_base scope lod_tensor ddim memory)
nv_library(nccl_all_reduce_op_handle SRCS nccl_all_reduce_op_handle.cc DEPS op_handle_base scope lod_tensor ddim memory
Expand All @@ -21,10 +21,10 @@ cc_library(ssa_graph_executor SRCS ssa_graph_executor.cc DEPS ssa_graph framewor
cc_library(threaded_ssa_graph_executor SRCS threaded_ssa_graph_executor.cc DEPS fetch_op_handle ssa_graph_executor scope
simple_threadpool device_context)

cc_library(broadcast_op_handle SRCS broadcast_op_handle.cc DEPS op_handle_base scope lod_tensor ddim memory)
cc_library(gather_op_handle SRCS gather_op_handle.cc DEPS op_handle_base scope lod_tensor ddim memory)
cc_library(broadcast_op_handle SRCS broadcast_op_handle.cc DEPS op_handle_base scope ddim memory)
cc_library(gather_op_handle SRCS gather_op_handle.cc DEPS op_handle_base scope ddim memory)

cc_test(broadcast_op_test SRCS broadcast_op_handle_test.cc DEPS var_handle op_handle_base scope lod_tensor ddim memory
cc_test(broadcast_op_test SRCS broadcast_op_handle_test.cc DEPS var_handle op_handle_base scope ddim memory
device_context broadcast_op_handle)
cc_test(gather_op_test SRCS gather_op_handle_test.cc DEPS var_handle op_handle_base scope lod_tensor ddim memory
cc_test(gather_op_test SRCS gather_op_handle_test.cc DEPS var_handle op_handle_base scope ddim memory
device_context gather_op_handle)
29 changes: 10 additions & 19 deletions paddle/fluid/framework/details/broadcast_op_handle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,16 @@ namespace paddle {
namespace framework {
namespace details {

static Tensor *GetTensorFromVar(Variable *in_var) {
if (in_var->IsType<LoDTensor>()) {
return in_var->GetMutable<LoDTensor>();
} else if (in_var->IsType<SelectedRows>()) {
return in_var->GetMutable<SelectedRows>()->mutable_value();
} else {
PADDLE_THROW("Var should be LoDTensor or SelectedRows");
}
return nullptr;
}
BroadcastOpHandle::BroadcastOpHandle(const std::vector<Scope *> &local_scopes,
const std::vector<platform::Place> &places)
: local_scopes_(local_scopes), places_(places) {}

void BroadcastOpHandle::RunImpl() {
PADDLE_ENFORCE_EQ(this->inputs_.size(), 1);
PADDLE_ENFORCE_EQ(this->outputs_.size(), places_.size());
PADDLE_ENFORCE_EQ(this->inputs_.size(), 1,
"The number of input should be one.");
PADDLE_ENFORCE_EQ(
this->outputs_.size(), places_.size(),
"The number of output should equal to the number of places.");

// Wait input done, this Wait is asynchronous operation
auto in_var_handle = static_cast<VarHandle *>(this->inputs_[0]);
Expand All @@ -43,7 +36,9 @@ void BroadcastOpHandle::RunImpl() {
inputs_[0]->generated_op_->Wait(dev_ctxes_[in_place]);

auto in_scope_idx = in_var_handle->scope_idx_;
PADDLE_ENFORCE_LT(in_scope_idx, local_scopes_.size(), "");
PADDLE_ENFORCE_LT(in_scope_idx, local_scopes_.size(),
"The input(%s) is not in the local_scopes.",
in_var_handle->name_);
auto in_var = local_scopes_[in_scope_idx]->FindVar(in_var_handle->name_);

Tensor *in_tensor = GetTensorFromVar(in_var);
Expand All @@ -56,12 +51,8 @@ void BroadcastOpHandle::RunImpl() {
"%s is not the the local_scopes ", out_handle->name_);
auto *s = local_scopes_[out_scope_idx];
auto out_var = s->FindVar(out_handle->name_);

PADDLE_ENFORCE_EQ(
out_var->Type(), in_var->Type(),
"The type of input and output is not equal. (%s_%d vs %s_%d)",
out_handle->name_, out_handle->scope_idx_, in_var_handle->name_,
in_var_handle->scope_idx_);
PADDLE_ENFORCE_EQ(out_p.which(), in_place.which(),
"The place of input and output should be the same.");

if (in_var->IsType<framework::SelectedRows>()) {
auto &in_sr = in_var->Get<framework::SelectedRows>();
Expand Down
4 changes: 0 additions & 4 deletions paddle/fluid/framework/details/broadcast_op_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ namespace paddle {
namespace framework {
namespace details {

/*
* Broadcast the input to all scope.
*
*/
struct BroadcastOpHandle : public OpHandleBase {
const std::vector<Scope *> &local_scopes_;
const std::vector<platform::Place> &places_;
Expand Down
18 changes: 13 additions & 5 deletions paddle/fluid/framework/details/broadcast_op_handle_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

#include "paddle/fluid/platform/device_context.h"

namespace paddle {
namespace framework {
namespace details {

namespace f = paddle::framework;
namespace p = paddle::platform;

Expand All @@ -25,7 +29,7 @@ const f::DDim kDims = {20, 20};

class BroadcastTester : public ::testing::Test {
public:
void InitCtx(bool use_gpu) {
void InitCtxOnGpu(bool use_gpu) {
if (use_gpu) {
#ifdef PADDLE_WITH_CUDA
int count = p::GetCUDADeviceCount();
Expand Down Expand Up @@ -200,23 +204,27 @@ class BroadcastTester : public ::testing::Test {
};

TEST_F(BroadcastTester, TestCPUBroadcastTestLodTensor) {
InitCtx(false);
InitCtxOnGpu(false);
TestBroadcastLodTensor();
}

TEST_F(BroadcastTester, TestCPUBroadcastTestSelectedRows) {
InitCtx(false);
InitCtxOnGpu(false);
TestBroadcastSelectedRows();
}

#ifdef PADDLE_WITH_CUDA
TEST_F(BroadcastTester, TestGPUBroadcastTestLodTensor) {
InitCtx(true);
InitCtxOnGpu(true);
TestBroadcastLodTensor();
}

TEST_F(BroadcastTester, TestGPUBroadcastTestSelectedRows) {
InitCtx(true);
InitCtxOnGpu(true);
TestBroadcastSelectedRows();
}
#endif

} // namespace details
} // namespace framework
} // namespace paddle
39 changes: 20 additions & 19 deletions paddle/fluid/framework/details/gather_op_handle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,16 @@ namespace paddle {
namespace framework {
namespace details {

static Tensor *GetTensorFromVar(Variable *in_var) {
if (in_var->IsType<LoDTensor>()) {
return in_var->GetMutable<LoDTensor>();
} else if (in_var->IsType<SelectedRows>()) {
return in_var->GetMutable<SelectedRows>()->mutable_value();
} else {
PADDLE_THROW("Var should be LoDTensor or SelectedRows");
}
return nullptr;
}
GatherOpHandle::GatherOpHandle(const std::vector<Scope *> &local_scopes,
const std::vector<platform::Place> &places)
: local_scopes_(local_scopes), places_(places) {}

void GatherOpHandle::RunImpl() {
PADDLE_ENFORCE_EQ(this->inputs_.size(), places_.size());
PADDLE_ENFORCE_EQ(this->outputs_.size(), 1);
PADDLE_ENFORCE_EQ(
this->inputs_.size(), places_.size(),
"The number of inputs should be equal to the number of place.");
PADDLE_ENFORCE_EQ(this->outputs_.size(), 1,
"The number of output should be one.");

// Wait input done, this Wait is asynchronous operation
for (auto *in : inputs_) {
Expand All @@ -46,6 +39,7 @@ void GatherOpHandle::RunImpl() {
auto in_0_handle = static_cast<VarHandle *>(inputs_[0]);
auto pre_in_var =
local_scopes_[in_0_handle->scope_idx_]->FindVar(in_0_handle->name_);
auto pre_place = in_0_handle->place_;

std::vector<int64_t> out_rows;
std::vector<Tensor *> in_tensors;
Expand All @@ -58,7 +52,8 @@ void GatherOpHandle::RunImpl() {
in_places.push_back(in_p);
PADDLE_ENFORCE_LT(in_handle->scope_idx_, local_scopes_.size(),
"%s is not the the local_scopes ", in_handle->name_);

PADDLE_ENFORCE_EQ(in_p.which(), pre_place.which(),
"The place of input should be the same.");
auto *s = local_scopes_[in_handle->scope_idx_];
auto in_var = s->FindVar(in_handle->name_);
PADDLE_ENFORCE_EQ(in_var->Type(), pre_in_var->Type(),
Expand All @@ -69,13 +64,17 @@ void GatherOpHandle::RunImpl() {
auto &in_sr = in_var->Get<framework::SelectedRows>();
auto in_sr_rows = in_sr.rows();
out_rows.insert(out_rows.begin(), in_sr_rows.begin(), in_sr_rows.end());
PADDLE_ENFORCE_EQ(pre_in.height(), in_sr.height(), "");
PADDLE_ENFORCE_EQ(pre_in.GetCompleteDims(), in_sr.GetCompleteDims(), "");
PADDLE_ENFORCE_EQ(pre_in.height(), in_sr.height(),
"The height of inputs is not consistent.");
PADDLE_ENFORCE_EQ(pre_in.GetCompleteDims(), in_sr.GetCompleteDims(), ,
"The dims of inputs is not consistent.");
} else if (in_var->IsType<framework::LoDTensor>()) {
auto &pre_in = pre_in_var->Get<framework::LoDTensor>();
auto &in_lodtensor = in_var->Get<framework::LoDTensor>();
PADDLE_ENFORCE_EQ(in_lodtensor.lod(), pre_in.lod());
PADDLE_ENFORCE_EQ(in_lodtensor.dims(), pre_in.dims());
PADDLE_ENFORCE_EQ(in_lodtensor.lod(), pre_in.lod(),
"The lod of inputs is not consistent.");
PADDLE_ENFORCE_EQ(in_lodtensor.dims(), pre_in.dims(),
"The dims of inputs is not consistent.");
} else {
PADDLE_THROW("Var should be LoDTensor or SelectedRows.");
}
Expand All @@ -88,7 +87,8 @@ void GatherOpHandle::RunImpl() {
auto &out_place = out_handle->place_;
auto out_scope_idx = out_handle->scope_idx_;
auto out_var = local_scopes_[out_scope_idx]->FindVar(out_handle->name_);

PADDLE_ENFORCE_EQ(out_place.which(), pre_place.which(),
"The place of input and output should be the same.");
if (pre_in_var->IsType<framework::SelectedRows>()) {
auto &pre_in = pre_in_var->Get<framework::SelectedRows>();
auto out = out_var->GetMutable<framework::SelectedRows>();
Expand All @@ -110,12 +110,13 @@ void GatherOpHandle::RunImpl() {
s = e;
}
} else if (pre_in_var->IsType<framework::LoDTensor>()) {
// gather LoDTensor ???
} else {
PADDLE_THROW("Var should be LoDTensor or SelectedRows.");
}
}

std::string GatherOpHandle::Name() const { return "broadcast"; }
std::string GatherOpHandle::Name() const { return "gather"; }
} // namespace details
} // namespace framework
} // namespace paddle
4 changes: 0 additions & 4 deletions paddle/fluid/framework/details/gather_op_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ namespace paddle {
namespace framework {
namespace details {

/*
* Broadcast the input to all scope.
*
*/
struct GatherOpHandle : public OpHandleBase {
const std::vector<Scope *> &local_scopes_;
const std::vector<platform::Place> &places_;
Expand Down
56 changes: 12 additions & 44 deletions paddle/fluid/framework/details/gather_op_handle_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

#include "paddle/fluid/platform/device_context.h"

namespace paddle {
namespace framework {
namespace details {
namespace f = paddle::framework;
namespace p = paddle::platform;

Expand All @@ -25,7 +28,7 @@ const f::DDim kDims = {20, 20};

class GatherTester : public ::testing::Test {
public:
void InitCtx(bool use_gpu) {
void InitCtxOnGpu(bool use_gpu) {
if (use_gpu) {
#ifdef PADDLE_WITH_CUDA
int count = p::GetCUDADeviceCount();
Expand Down Expand Up @@ -103,45 +106,7 @@ class GatherTester : public ::testing::Test {
}
}

void TestGatherLodTensor() {
// int input_scope_idx = 0;
// InitGatherOp<f::LoDTensor>(input_scope_idx);
//
// auto in_var = local_scope_[input_scope_idx]->Var("input");
// auto in_lod_tensor = in_var->GetMutable<f::LoDTensor>();
// in_lod_tensor->mutable_data<float>(kDims, gpu_list_[input_scope_idx]);
//
// std::vector<float> send_vector(f::product(kDims), input_scope_idx +
// 12);
// for (size_t k = 0; k < send_vector.size(); ++k) {
// send_vector[k] = k;
// }
// f::LoD lod{{0, 10, 20}};
// paddle::framework::TensorFromVector<float>(
// send_vector, *(ctxs_[input_scope_idx]), in_lod_tensor);
// in_lod_tensor->set_lod(lod);
//
// gather_op_handle_->Run(false);
//
// WaitAll();
//
// p::CPUPlace cpu_place;
// for (size_t j = 0; j < gpu_list_.size(); ++j) {
// auto out_var = local_scope_[j]->Var("out");
// auto out_tensor = out_var->Get<f::LoDTensor>();
// PADDLE_ENFORCE_EQ(out_tensor.lod(), lod, "lod is not equal.");
//
// f::Tensor result_tensor;
// f::TensorCopy(out_tensor, cpu_place, *(ctxs_[j]), &result_tensor);
// float* ct = result_tensor.mutable_data<float>(cpu_place);
//
// for (int64_t j = 0; j < f::product(kDims); ++j) {
// ASSERT_NEAR(ct[j], send_vector[j], 1e-5);
// }
// }
//
// GatherOpDestroy();
}
void TestGatherLodTensor() {}

void TestGatherSelectedRows() {
int output_scope_idx = 0;
Expand Down Expand Up @@ -205,23 +170,26 @@ class GatherTester : public ::testing::Test {
};

// TEST_F(GatherTester, TestCPUGatherTestLodTensor) {
// InitCtx(false);
// InitCtxOnGpu(false);
// TestGatherLodTensor();
//}

TEST_F(GatherTester, TestCPUGatherTestSelectedRows) {
InitCtx(false);
InitCtxOnGpu(false);
TestGatherSelectedRows();
}

#ifdef PADDLE_WITH_CUDA
// TEST_F(GatherTester, TestGPUGatherTestLodTensor) {
// InitCtx(true);
// InitCtxOnGpu(true);
// TestGatherLodTensor();
//}

TEST_F(GatherTester, TestGPUGatherTestSelectedRows) {
InitCtx(true);
InitCtxOnGpu(true);
TestGatherSelectedRows();
}
#endif
} // namespace details
} // namespace framework
} // namespace paddle
15 changes: 15 additions & 0 deletions paddle/fluid/framework/details/op_handle_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,21 @@
namespace paddle {
namespace framework {
namespace details {

// GetTensorFromVar is used in broadcast_op handle and gather_op handle, so it
// should be placed in a commonplace. I don't find an appropriate place, so I
// temporarily place it in op_handle_base.
Tensor *GetTensorFromVar(Variable *in_var) {
if (in_var->IsType<LoDTensor>()) {
return in_var->GetMutable<LoDTensor>();
} else if (in_var->IsType<SelectedRows>()) {
return in_var->GetMutable<SelectedRows>()->mutable_value();
} else {
PADDLE_THROW("Var should be LoDTensor or SelectedRows");
}
return nullptr;
}

std::string OpHandleBase::DebugString() const {
std::stringstream ss;
ss << "(";
Expand Down
8 changes: 8 additions & 0 deletions paddle/fluid/framework/details/op_handle_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,21 @@
#include <vector>

#include "paddle/fluid/framework/details/var_handle.h"
#include "paddle/fluid/framework/lod_tensor.h"
#include "paddle/fluid/framework/selected_rows.h"
#include "paddle/fluid/framework/variable.h"
#include "paddle/fluid/platform/device_context.h"
#include "paddle/fluid/platform/macros.h"

namespace paddle {
namespace framework {
namespace details {

// GetTensorFromVar is used in broadcast_op handle and gather_op handle, so it
// should be placed in a commonplace. I don't find an appropriate place, so I
// temporarily place it in op_handle.
Tensor *GetTensorFromVar(Variable *in_var);

class OpHandleBase {
private:
DISABLE_COPY_AND_ASSIGN(OpHandleBase);
Expand Down

0 comments on commit e26c6d7

Please sign in to comment.