Skip to content
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

operator design #2555

Closed
wants to merge 11 commits into from
Closed

Conversation

jacquesqiao
Copy link
Member

@jacquesqiao jacquesqiao commented Jun 22, 2017

refs:

Design doc for operator attribute(#2606)

namespace framework {

template <class DeviceContext>
class Operator {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我们的Operator需要几层来去定义呢?
caffe2的做法是三层:

第一层是OperatorBase,用来做类型擦除

class OperatorBase

第二层是模板类Operator

template <typename Context>
class Operator

第三层是具体的实现

template <typename T, typename Context>
class ReluOp final : Operator<Context>

之所以Caffe2是三层设计,因为在Operator里面有一个成员变量device_context_,但目前看起来这个变量并没有什么作用。

或许在这里只需要两层设计:

第一层,OperatorBase作为类型擦除

class OperatorBase {
  bool Run(Scope* scope) final {
    return RunOnDevice();
  }
  virtual bool RunOnDevice() = 0;
protected:
  vector<string*> inputs_;
  vector<string*> outputs_;
};

第二层为具体的实现:

template <typename T, typename Context>
class ReluOp : final OperatorBase {
};

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context 这个概念是不是需要专门看一下,来帮助我们理解如何正确的设计Op

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是的,我觉得需要再看一下
如果我们需要的仅仅是表示一个Op是CPU/GPU设备上,那么简单的 class Device就行了;
如果需要在不同设备上有些特殊操作之类的,那么可能会需要一个略为复杂的class Context

};

// Operator is the class your should derive when implement a new Operator.
template <typename T>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename T-> DeviceContext is better.

explicit Operator(const OperatorDesc& desc): OperatorBase(desc) {}

// This function will
Error Run(Scope* scope, T* context) const final {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please give a pseudo implementation here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Error Run(Scope* scope, T* context) const final {}

// when implement an Op, your should implement this function.
virtual Error Run(std::vector<Variable> inputs,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const std::vector<const Variable*>& inputs, const std::vector<Variable*>& outputs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

`OperatorBase` cantains the base element of an Operator, but without any specialized information. The reason is that Net can treat all Ops as OperatorBase and donot need to consider the type or context of these ops. It can just call run() of Op:

```cpp
class Net {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Net => NetworkBase for consistent to Network design


#### `OperatorBase`

`OperatorBase` cantains the base element of an Operator, but without any specialized information. The reason is that Net can treat all Ops as OperatorBase and donot need to consider the type or context of these ops. It can just call run() of Op:
Copy link
Collaborator

@reyoung reyoung Jun 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contains --> is a base class of an Operator
base element --> base class
cantains --> contains
specialized --> specific

The reason is that --> because
Net --> NetworkBase
donot --> do not
run --> Run


`Operator` has a template parameter `DeviceContext`. DeviceContext is used to specify on which device this op will run. Each Op will implement multi Op according to different Context.

#### We design the Operator class with tree layer.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tree -> three ?

if (scale_ <= 0.0f) {
return Error("Scale of cosine op should be larger than 0.0");
}
return Error(); // OK;
Copy link
Contributor

@Superjomn Superjomn Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename Error -> Status better ?
It's wired that Error() means ok, and Error(xxx) means a real error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants