-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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.
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
Feature/extract op info into op info.cc #3587
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. */ | ||
|
||
#include "paddle/framework/op_info.h" | ||
|
||
namespace paddle { | ||
namespace framework { | ||
|
||
static OpInfoMap* g_op_info_map = nullptr; | ||
|
||
OpInfoMap& OpInfoMap::Instance() { | ||
if (g_op_info_map == nullptr) { | ||
g_op_info_map = new OpInfoMap(); | ||
} | ||
return *g_op_info_map; | ||
} | ||
} // namespace framework | ||
} // namespace paddle |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. */ | ||
|
||
#pragma once | ||
#include <functional> | ||
#include <map> | ||
#include <string> | ||
#include <unordered_map> | ||
|
||
#include "paddle/framework/attribute.h" | ||
|
||
namespace paddle { | ||
namespace framework { | ||
class OperatorBase; | ||
using VariableNameMap = std::map<std::string, std::vector<std::string>>; | ||
|
||
using OpCreator = std::function<OperatorBase*( | ||
const std::string& /*type*/, const VariableNameMap& /*inputs*/, | ||
const VariableNameMap& /*outputs*/, const AttributeMap& /*attrs*/)>; | ||
|
||
struct OpInfo { | ||
OpCreator creator_; | ||
std::string grad_op_type_; | ||
OpProto* proto_; | ||
OpAttrChecker* checker_; | ||
|
||
bool HasOpProtoAndChecker() const { | ||
return proto_ != nullptr && checker_ != nullptr; | ||
} | ||
|
||
const OpProto& Proto() const { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if these PADDLE_ENFORCE are necessary. But if they are, please make OpInfo a class instead of a struct due to https://google.github.io/styleguide/cppguide.html#Structs_vs._Classes. Especially, if we need OpInfo a class, we should declare creator_ and other data members |
||
PADDLE_ENFORCE_NOT_NULL(proto_, "Operator Proto has not been registered"); | ||
PADDLE_ENFORCE(proto_->IsInitialized(), | ||
"Operator Proto must be initialized in op info"); | ||
return *proto_; | ||
} | ||
|
||
const OpAttrChecker& Checker() const { | ||
PADDLE_ENFORCE_NOT_NULL(checker_, | ||
"Operator Checker has not been registered"); | ||
return *checker_; | ||
} | ||
|
||
const OpCreator& Creator() const { | ||
PADDLE_ENFORCE_NOT_NULL(creator_, | ||
"Operator Creator has not been registered"); | ||
return creator_; | ||
} | ||
|
||
bool HasGradientOp() const { return !grad_op_type_.empty(); } | ||
}; | ||
|
||
class OpInfoMap { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class seems unnecessary and should be removed, as its methods basically duplicate unordered_map's interface. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are lot duplicated code before. See code here. To use |
||
public: | ||
static OpInfoMap& Instance(); | ||
|
||
OpInfoMap(const OpInfoMap& o) = delete; | ||
OpInfoMap(OpInfoMap&& o) = delete; | ||
OpInfoMap& operator=(const OpInfoMap& o) = delete; | ||
OpInfoMap& operator=(OpInfoMap&& o) = delete; | ||
|
||
bool Has(const std::string& op_type) const { | ||
return map_.find(op_type) != map_.end(); | ||
} | ||
|
||
void Insert(const std::string& type, const OpInfo& info) { | ||
PADDLE_ENFORCE(!Has(type), "Operator %s has been registered", type); | ||
map_.insert({type, info}); | ||
} | ||
|
||
const OpInfo& Get(const std::string& type) const { | ||
auto it = map_.find(type); | ||
PADDLE_ENFORCE(it != map_.end(), "Operator %s are not found", type); | ||
return it->second; | ||
} | ||
|
||
template <typename Callback> | ||
void IterAllInfo(Callback callback) { | ||
for (auto& it : map_) { | ||
callback(it.first, it.second); | ||
} | ||
} | ||
|
||
private: | ||
OpInfoMap() = default; | ||
std::unordered_map<std::string, const OpInfo> map_; | ||
}; | ||
|
||
} // namespace framework | ||
} // namespace paddle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The right way to implement a singleton in C++ doesn't need global pointer variables:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables
In Google C++ style, Only POD type can be a global variable.
OpInfoMap
is a class, it should not be a global variable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
确实。不过看上去也不需要一个global variable,可以是这样?