-
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
scope design doc #2548
scope design doc #2548
Changes from 3 commits
bcac91a
002a6c9
674b1d3
7a48507
3e09978
acf0b75
04ad9b6
581e4c1
0b70361
76e2a3c
8282138
4413726
d7aca77
2d5507f
64a1cdf
73b1c5b
1f0056b
0b07583
17eed33
37fd48b
c3a4b8b
db96c0e
f104ce2
32fe097
eab0e52
63a56b4
921fa13
5d88249
f8a209c
c5ad89a
237efc2
3bac2d0
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,58 @@ | ||
# Scope | ||
|
||
### Define | ||
|
||
Scope is a context to manage Variables. It mainly contains a map from Variable name to Variable. Net will get and update variable throw scope. | ||
|
||
```cpp | ||
class Variable; | ||
using VariablePtr = std::shared_ptr<Variable>; | ||
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. Also, Caffe2 is using typedef CaffeMap<string, unique_ptr<Blob> > BlobMap; Why do we use To my understand, 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. When using I think to use Maybe we should have an agreement. We cannot store any variable pointer in our code. 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. Let us don't save pointers from unique_ptr as a class data member. I don't think |
||
|
||
class Scope final { | ||
public: | ||
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. Just a reminder -- I've put .clang-format files that strictly follows Google style in new source directories 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. done |
||
Scope(); | ||
Scope(const std::shared_ptr<Scope>& parent); | ||
|
||
//! Get Variable in this scope. | ||
//! @return nullptr if no such variable. | ||
const VariablePtr& getVar(const std::string& name) 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. Here the return type must be Also, following Google code style, this method should be named |
||
|
||
//! Create or get a variable in this scope. | ||
VariablePtr& createOrGetVar(const std::string& name); | ||
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. Maybe rename it into 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. done |
||
|
||
private: | ||
/// variable name -> variable | ||
std::unordered_map<std::string, VariablePtr> vars_; | ||
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. Why Anyway, I noticed that Caffe2 is using private:
BlobMap blob_map_; where typedef CaffeMap<string, unique_ptr<Blob> > BlobMap; and using CaffeMap = std::map<Key, Value>;
// using CaffeMap = std::unordered_map; 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. yangqing has metioned why he prefer
|
||
std::shared_ptr<Scope> parent_{nullptr}; | ||
}; | ||
``` | ||
|
||
You need to specify a scope to run a Net. One net can run in different scopes and update different variable in the scope. If you did not specify one, It will run in a default scope. | ||
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. Agreed! I second removing the requirement that a net belongs to a workspace and am happy to see that our Scope doesn't have a line like the one in class Workspace {
...
NetMap net_map_; |
||
|
||
```cpp | ||
Scope global; | ||
auto x = newVar("X"); // x is created in scope global, implicitly. | ||
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. Just a remind -- the following functions/methods do not following Google naming style:
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. done |
||
auto y = newVar("Y"); | ||
Net net1; | ||
net1.addOp("add", {x, y}, {x}); // x = x + y; | ||
net1.run(); | ||
|
||
for (size_t i=0; i<10; ++i) { | ||
Scope local; | ||
auto tmp = newVar("tmp"); // tmp is created in scope local. | ||
Net net2; | ||
net2.addOp("add", {x, y}, {tmp}); | ||
net2.run(); // tmp = x + y; | ||
} | ||
|
||
Net net3; | ||
net3.addOp("add", {x, y}, {"tmp"}); // error! cannot found "tmp" in global scope. | ||
|
||
``` | ||
|
||
### Chain structure | ||
|
||
Scope has a pointer point to it's parent scope, this is mainly used in RNN when it need to create many stepNet. | ||
|
||
|
||
### Scope Guard |
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.
I don't think we need this using alias because in a different scenario we might use different smart pointers. I'd suggest that we write everything in the definition of
vars_
like: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.
Maybe we should have a global header called
TypeDefs.h
. In this header, we specify all default types forMap
,String
,Set
, etc. LikeThere 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.
Let us do such kind of summarization later -- after we make sure that some types are common to various situations.