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

Evaluator accumulate #4828

Merged
merged 2 commits into from
Nov 2, 2017
Merged

Conversation

typhoonzero
Copy link
Contributor

Fix #3808


class TestEvaluator(unittest.TestCase):
def setup(self, scope, inputs, outputs):
def __create_var__(var_name, arr):
Copy link
Contributor

Choose a reason for hiding this comment

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

def __create_var__ -> def _create_var.

class Evaluator(object):
def __init__(self,
scope,
operator='accuracy',
Copy link
Contributor

Choose a reason for hiding this comment

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

看了下这个Evaluator里只有对多个mini-batch间的结果累加求平均。但对于PrecisionRecall这种多个mini-batch间结果统计并不是简单的累加求平均。

所以感觉如果在Python里抽象出Evaluator,用来统计多个mini-batch间的计算结果,需要抽象出一个Evaluator基类接口,然后有很多子类,AccuracyEvalutor, PrecisionRecallEvalutor等等。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

嗯,前面定义了avg_accumulate,是解决这个问题的。但的确没有考虑编译时。需要把avg accumulator等作为op,才可以使python定义的计算是编译时的。

@helinwang
Copy link
Contributor

helinwang commented Oct 23, 2017

@qingqing01 @typhoonzero @wangkuiyi from my understanding the PaddlePaddle program is divided into compile time (currently Python) and runtime (C++): the Python code generates a protobuf which describes the computation, and the C++ code runs the computation description.

This implementation of the executor is in Python (uses scope which belongs to the runtime), I am not sure if it fits with the compile time and runtime concept. I think this requirement is very important for every OP implementation, @qingqing01 @wangkuiyi perhaps we need to make sure that everyone is on the same page.

@typhoonzero
Copy link
Contributor Author

@helinwang you are right, I'll try to put this implementation to compile time.

This was referenced Oct 27, 2017
var_map[input] = [input]
var_map[label] = [label]
var_map[output] = [output]
self.op = op.Operator(operator, **var_map)
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently, we can not create operator immediately out of Python Block and Program.

class Program(object):
def __init__(self):
self.desc = core.ProgramDesc()
self.blocks = [Block(self, 0)]
self.current_block_idx = 0
def __str__(self):
protostr = self.desc.serialize_to_string()
proto = framework_pb2.ProgramDesc.FromString(str(protostr))
return proto.__str__()
def clone(self):
p = Program()
p.desc = core.ProgramDesc(self.desc)
p.blocks = [Block(p, i) for i in xrange(self.desc.num_blocks())]
p.sync_with_cpp()
return p

May I just merge this work into codebase and let me polish it to fit in the refactorization code? : )
Our book porting work need this feature eagerly. @typhoonzero @qingqing01

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, Thanks. Please also refer to some of my thoughts: #5157

Copy link
Contributor

@dzhwinter dzhwinter left a comment

Choose a reason for hiding this comment

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

LGTM!
I just merge this work into codebase and let me polish it.
Our book chapters need this feature eagerly.
@typhoonzero @helinwang

@dzhwinter dzhwinter merged commit 08ca726 into PaddlePaddle:develop Nov 2, 2017
@typhoonzero typhoonzero deleted the evaluator_pybind branch December 22, 2017 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants