From f25eb9a71d31be1889dafcf62aafac72980954e2 Mon Sep 17 00:00:00 2001 From: Xin Pan Date: Tue, 6 Nov 2018 13:21:10 +0800 Subject: [PATCH] fix some tests. test=develop --- .../details/broadcast_op_handle_test.h | 52 ++++++++++--------- .../framework/details/fetch_op_handle.cc | 6 +-- .../details/fused_broadcast_op_handle_test.cc | 34 ++++++------ .../details/gather_op_handle_test.cc | 26 +++++----- .../details/multi_devices_graph_check_pass.cc | 5 +- .../framework/details/multi_devices_helper.h | 2 +- .../details/reduce_op_handle_test.cc | 4 +- 7 files changed, 65 insertions(+), 64 deletions(-) diff --git a/paddle/fluid/framework/details/broadcast_op_handle_test.h b/paddle/fluid/framework/details/broadcast_op_handle_test.h index 1a2a9ac328c..4305eb65733 100644 --- a/paddle/fluid/framework/details/broadcast_op_handle_test.h +++ b/paddle/fluid/framework/details/broadcast_op_handle_test.h @@ -37,8 +37,9 @@ struct TestBroadcastOpHandle { std::vector local_scopes_; std::vector param_scopes_; Scope g_scope_; - std::unique_ptr op_handle_; - std::vector> vars_; + OpHandleBase* op_handle_; + std::vector vars_; + std::vector> nodes_; std::vector place_list_; bool use_gpu_; #ifdef PADDLE_WITH_CUDA @@ -90,6 +91,7 @@ struct TestBroadcastOpHandle { } void InitBroadcastOp(size_t input_scope_idx) { + nodes_.clear(); for (size_t j = 0; j < place_list_.size(); ++j) { local_scopes_.push_back(&(g_scope_.NewScope())); Scope& local_scope = local_scopes_.back()->NewScope(); @@ -101,39 +103,39 @@ struct TestBroadcastOpHandle { } param_scopes_[input_scope_idx]->Var("input"); - std::unique_ptr n = - ir::CreateNodeForTest("node0", ir::Node::Type::kOperation); + nodes_.emplace_back( + ir::CreateNodeForTest("node0", ir::Node::Type::kOperation)); if (use_gpu_) { #ifdef PADDLE_WITH_CUDA - op_handle_.reset(new BroadcastOpHandle(n.get(), local_scopes_, - place_list_, nccl_ctxs_.get())); + op_handle_ = new BroadcastOpHandle(nodes_.back().get(), local_scopes_, + place_list_, nccl_ctxs_.get()); #else PADDLE_THROW("CUDA is not support."); #endif } else { #ifdef PADDLE_WITH_CUDA - op_handle_.reset(new BroadcastOpHandle(n.get(), local_scopes_, - place_list_, nccl_ctxs_.get())); + op_handle_ = new BroadcastOpHandle(nodes_.back().get(), local_scopes_, + place_list_, nccl_ctxs_.get()); #else - op_handle_.reset( - new BroadcastOpHandle(n.get(), local_scopes_, place_list_)); + op_handle_ = new BroadcastOpHandle(nodes_.back().get(), local_scopes_, + place_list_); #endif } - std::unique_ptr v = - ir::CreateNodeForTest("node1", ir::Node::Type::kVariable); - auto* in_var_handle = new VarHandle(v.get(), 1, input_scope_idx, "input", - place_list_[input_scope_idx]); + nodes_.emplace_back( + ir::CreateNodeForTest("node1", ir::Node::Type::kVariable)); + auto* in_var_handle = new VarHandle(nodes_.back().get(), 1, input_scope_idx, + "input", place_list_[input_scope_idx]); vars_.emplace_back(in_var_handle); op_handle_->AddInput(in_var_handle); // add dummy var - std::unique_ptr v2 = - ir::CreateNodeForTest("node2", ir::Node::Type::kVariable); - vars_.emplace_back(new DummyVarHandle(v2.get())); + nodes_.emplace_back( + ir::CreateNodeForTest("node2", ir::Node::Type::kVariable)); + vars_.emplace_back(new DummyVarHandle(nodes_.back().get())); DummyVarHandle* dummy_var_handle = - static_cast(vars_.back().get()); + static_cast(vars_.back()); dummy_var_handle->ClearGeneratedOp(); op_handle_->AddInput(dummy_var_handle); @@ -141,20 +143,20 @@ struct TestBroadcastOpHandle { if (!use_gpu_) { op_handle_->SetDeviceContext(place_list_[j], ctxs_[j].get()); } - std::unique_ptr v3 = - ir::CreateNodeForTest("node3", ir::Node::Type::kVariable); + nodes_.emplace_back( + ir::CreateNodeForTest("node3", ir::Node::Type::kVariable)); VarHandle* out_var_handle = - new VarHandle(v3.get(), 2, j, "out", place_list_[j]); + new VarHandle(nodes_.back().get(), 2, j, "out", place_list_[j]); vars_.emplace_back(out_var_handle); op_handle_->AddOutput(out_var_handle); } // add dummy var - std::unique_ptr v4 = - ir::CreateNodeForTest("node4", ir::Node::Type::kVariable); - vars_.emplace_back(new DummyVarHandle(v4.get())); + nodes_.emplace_back( + ir::CreateNodeForTest("node4", ir::Node::Type::kVariable)); + vars_.emplace_back(new DummyVarHandle(nodes_.back().get())); DummyVarHandle* out_dummy_var_handle = - static_cast(vars_.back().get()); + static_cast(vars_.back()); out_dummy_var_handle->ClearGeneratedOp(); op_handle_->AddOutput(out_dummy_var_handle); } diff --git a/paddle/fluid/framework/details/fetch_op_handle.cc b/paddle/fluid/framework/details/fetch_op_handle.cc index fe18b2060c5..648adae06fa 100644 --- a/paddle/fluid/framework/details/fetch_op_handle.cc +++ b/paddle/fluid/framework/details/fetch_op_handle.cc @@ -28,11 +28,7 @@ FetchOpHandle::FetchOpHandle(ir::Node *node, FeedFetchList *data, size_t offset, offset_(offset), local_scopes_(local_scopes) {} -FetchOpHandle::~FetchOpHandle() { - for (auto *input_var : inputs_) { - input_var->RemoveOutput(this, this->Node()); - } -} +FetchOpHandle::~FetchOpHandle() {} void FetchOpHandle::RecordWaitEventOnCtx(platform::DeviceContext *waited_ctx) { PADDLE_THROW("Nobody should wait FetchOp. Unexpceted Error"); diff --git a/paddle/fluid/framework/details/fused_broadcast_op_handle_test.cc b/paddle/fluid/framework/details/fused_broadcast_op_handle_test.cc index 0f12bd2b4e8..541993c7433 100644 --- a/paddle/fluid/framework/details/fused_broadcast_op_handle_test.cc +++ b/paddle/fluid/framework/details/fused_broadcast_op_handle_test.cc @@ -22,8 +22,10 @@ namespace details { struct TestFusedBroadcastOpHandle : TestBroadcastOpHandle { std::vector out_varnames_; + std::vector> nodes_; void InitFusedBroadcastOp(std::vector input_scope_idxes) { + nodes_.clear(); // initialize scope and var for (size_t i = 0; i < place_list_.size(); ++i) { local_scopes_.push_back(&(g_scope_.NewScope())); @@ -39,41 +41,41 @@ struct TestFusedBroadcastOpHandle : TestBroadcastOpHandle { } // create op handle node - std::unique_ptr n = - ir::CreateNodeForTest("fused_broadcast", ir::Node::Type::kOperation); + nodes_.emplace_back( + ir::CreateNodeForTest("fused_broadcast", ir::Node::Type::kOperation)); if (use_gpu_) { #ifdef PADDLE_WITH_CUDA - op_handle_.reset(new FusedBroadcastOpHandle( - n.get(), local_scopes_, place_list_, nccl_ctxs_.get())); + op_handle_ = new FusedBroadcastOpHandle( + nodes_.back().get(), local_scopes_, place_list_, nccl_ctxs_.get()); #else PADDLE_THROW("CUDA is not supported."); #endif } else { #ifdef PADDLE_WITH_CUDA - op_handle_.reset(new FusedBroadcastOpHandle( - n.get(), local_scopes_, place_list_, nccl_ctxs_.get())); + op_handle_ = new FusedBroadcastOpHandle( + nodes_.back().get(), local_scopes_, place_list_, nccl_ctxs_.get()); #else - op_handle_.reset( - new FusedBroadcastOpHandle(n.get(), local_scopes_, place_list_)); + op_handle_ = new FusedBroadcastOpHandle(nodes_.back().get(), + local_scopes_, place_list_); #endif } for (size_t i = 0; i < input_scope_idxes.size(); ++i) { // add input var handle - std::unique_ptr in_node = - ir::CreateNodeForTest("in_node" + i, ir::Node::Type::kVariable); + nodes_.emplace_back( + ir::CreateNodeForTest("in_node" + i, ir::Node::Type::kVariable)); VarHandle* in_var_handle = - new VarHandle(in_node.get(), 1, input_scope_idxes[i], "in_var" + i, - place_list_[input_scope_idxes[i]]); + new VarHandle(nodes_.back().get(), 1, input_scope_idxes[i], + "in_var" + i, place_list_[input_scope_idxes[i]]); vars_.emplace_back(in_var_handle); op_handle_->AddInput(in_var_handle); // add output var handle for (size_t j = 0; j < place_list_.size(); ++j) { - std::unique_ptr out_node = - ir::CreateNodeForTest("out_node" + i, ir::Node::Type::kVariable); - VarHandle* out_var_handle = - new VarHandle(out_node.get(), 2, j, "out_var" + i, place_list_[j]); + nodes_.emplace_back( + ir::CreateNodeForTest("out_node" + i, ir::Node::Type::kVariable)); + VarHandle* out_var_handle = new VarHandle( + nodes_.back().get(), 2, j, "out_var" + i, place_list_[j]); vars_.emplace_back(out_var_handle); op_handle_->AddOutput(out_var_handle); } diff --git a/paddle/fluid/framework/details/gather_op_handle_test.cc b/paddle/fluid/framework/details/gather_op_handle_test.cc index c83804e2626..e8cb7feb8be 100644 --- a/paddle/fluid/framework/details/gather_op_handle_test.cc +++ b/paddle/fluid/framework/details/gather_op_handle_test.cc @@ -34,6 +34,7 @@ struct TestGatherOpHandle { OpHandleBase* op_handle_; std::vector vars_; std::vector gpu_list_; + std::vector> nodes_; void WaitAll() { for (size_t j = 0; j < ctxs_.size(); ++j) { @@ -70,7 +71,7 @@ struct TestGatherOpHandle { } void InitGatherOp(size_t input_scope_idx) { - std::vector> nodes; + nodes_.clear(); for (size_t j = 0; j < gpu_list_.size(); ++j) { local_scopes_.push_back(&(g_scope_.NewScope())); Scope& local_scope = local_scopes_.back()->NewScope(); @@ -82,42 +83,43 @@ struct TestGatherOpHandle { } param_scopes_[input_scope_idx]->Var("out"); - nodes.emplace_back( + nodes_.emplace_back( ir::CreateNodeForTest("node", ir::Node::Type::kOperation).release()); op_handle_ = - new GatherOpHandle(nodes.back().get(), local_scopes_, gpu_list_); + new GatherOpHandle(nodes_.back().get(), local_scopes_, gpu_list_); // add input for (size_t j = 0; j < gpu_list_.size(); ++j) { op_handle_->SetDeviceContext(gpu_list_[j], ctxs_[j].get()); - nodes.emplace_back( + nodes_.emplace_back( ir::CreateNodeForTest("node1", ir::Node::Type::kVariable).release()); auto* in_var_handle = - new VarHandle(nodes.back().get(), 1, j, "input", gpu_list_[j]); + new VarHandle(nodes_.back().get(), 1, j, "input", gpu_list_[j]); vars_.emplace_back(in_var_handle); op_handle_->AddInput(in_var_handle); } // add dummy var - nodes.emplace_back( + nodes_.emplace_back( ir::CreateNodeForTest("node2", ir::Node::Type::kVariable).release()); - vars_.emplace_back(new DummyVarHandle(nodes.back().get())); + vars_.emplace_back(new DummyVarHandle(nodes_.back().get())); DummyVarHandle* in_dummy_var_handle = static_cast(vars_.back()); in_dummy_var_handle->ClearGeneratedOp(); op_handle_->AddInput(in_dummy_var_handle); // add output - nodes.emplace_back( + nodes_.emplace_back( ir::CreateNodeForTest("node3", ir::Node::Type::kVariable).release()); - auto* out_var_handle = new VarHandle(nodes.back().get(), 2, input_scope_idx, - "out", gpu_list_[input_scope_idx]); + auto* out_var_handle = + new VarHandle(nodes_.back().get(), 2, input_scope_idx, "out", + gpu_list_[input_scope_idx]); vars_.emplace_back(out_var_handle); op_handle_->AddOutput(out_var_handle); // add dummy var - nodes.emplace_back( + nodes_.emplace_back( ir::CreateNodeForTest("node4", ir::Node::Type::kVariable).release()); - vars_.emplace_back(new DummyVarHandle(nodes.back().get())); + vars_.emplace_back(new DummyVarHandle(nodes_.back().get())); DummyVarHandle* dummy_var_handle = static_cast(vars_.back()); op_handle_->AddOutput(dummy_var_handle); diff --git a/paddle/fluid/framework/details/multi_devices_graph_check_pass.cc b/paddle/fluid/framework/details/multi_devices_graph_check_pass.cc index 220aa88f7b0..5b03e9f9604 100644 --- a/paddle/fluid/framework/details/multi_devices_graph_check_pass.cc +++ b/paddle/fluid/framework/details/multi_devices_graph_check_pass.cc @@ -15,6 +15,7 @@ #include "paddle/fluid/framework/details/multi_devices_graph_check_pass.h" #include #include "paddle/fluid/framework/ir/graph.h" +#include "paddle/fluid/framework/ir/graph_helper.h" namespace paddle { namespace framework { @@ -45,9 +46,7 @@ bool SSAGraghBuilderWithChecker::IsValidGraph(const ir::Graph *graph) const { insert_pending_var(var); } - for (ir::Node *node : graph->Nodes()) { - if (!node->IsWrappedBy()) continue; - OpHandleBase *op = &node->Wrapper(); + for (OpHandleBase *op : ir::GetFilteredNodes(*graph)) { if (op->Inputs().empty()) { ready_ops.insert(op); } else { diff --git a/paddle/fluid/framework/details/multi_devices_helper.h b/paddle/fluid/framework/details/multi_devices_helper.h index 5a9e06369d9..1a2b75fbc0c 100644 --- a/paddle/fluid/framework/details/multi_devices_helper.h +++ b/paddle/fluid/framework/details/multi_devices_helper.h @@ -35,7 +35,7 @@ namespace details { // The outside vector is the device vector. Each element of this vector is a // map from variable name to variables. The variables, who have the same name, // will have a differsent version. The offset in the -// `std::vector>` is the version of varaibles. +// `std::vector` is the version of varaibles. typedef std::vector>> GraphVars; const char kGraphVars[] = "vars"; diff --git a/paddle/fluid/framework/details/reduce_op_handle_test.cc b/paddle/fluid/framework/details/reduce_op_handle_test.cc index 3a9a5841239..72299c0bfa9 100644 --- a/paddle/fluid/framework/details/reduce_op_handle_test.cc +++ b/paddle/fluid/framework/details/reduce_op_handle_test.cc @@ -30,8 +30,8 @@ struct TestReduceOpHandle { Scope g_scope_; std::vector local_scopes_; std::vector param_scopes_; - std::unique_ptr op_handle_; - std::vector> vars_; + OpHandleBase *op_handle_; + std::vector vars_; std::vector gpu_list_; std::vector> ctxs_; -- GitLab