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

Feature/enhance evaluator #5824

Merged
merged 9 commits into from
Nov 27, 2017

Conversation

reyoung
Copy link
Collaborator

@reyoung reyoung commented Nov 22, 2017

  1. Make Evaluator invoke layers.py, simplify the implementation
  2. Add comments
  3. remove the accuracy method, since users can directly new a Accuarcy class

Fix #5825

@reyoung reyoung added this to the Release 0.11.0 milestone Nov 23, 2017
dzhwinter
dzhwinter previously approved these changes Nov 23, 2017
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.

Looks much better than the previous implementation. If we go this way, there are two concerns.

  1. If a user wants to custom an evaluator, he needs to modify the layers and the evaluator.
  2. layers will be a core library in Python, it will be explosion increased after we complete it.

We can merge this PR and make that decision offline.

dzhwinter
dzhwinter previously approved these changes Nov 24, 2017
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.

Looks good.

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

@reyoung reyoung merged commit a619695 into PaddlePaddle:develop Nov 27, 2017
@reyoung reyoung deleted the feature/enhance_evaluator branch December 26, 2017 09:28
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.

2 participants