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

Contribute and logging #5181

Merged
merged 5 commits into from
Oct 30, 2017

Conversation

wangkuiyi
Copy link
Collaborator

This PR

  1. moves the new content to be merged in Create vlog_guide.md #5121 into /CONTRIBUTE.md,
  2. updates and merges content from doc/.../contribute_to_paddle_en.md into /CONTRIBUTE.md, and
  3. removes doc/.../contribute_to_paddle_cn.md.

CONTRIBUTING.md Outdated
When we run a PaddlePaddle application or test, we can specify a verbose threshold. For example:

```bash
GLOG_v=3 python ../python/paddle/v2/framework/tests/test_recurrent_op.py

Choose a reason for hiding this comment

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

add a module example GLOG_vmodule=buddy_allocator=2 GLOG_v=10 python ../python/paddle/v2/framework/tests/test_recurrent_op.py. This will set the vlog level of buddy_allocator to be 2 and the rest to be 10

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

CONTRIBUTING.md Outdated
| 1 | reserved |
| 2 | DL model compilation, including the building of backward pass and adding optimizer, feed, init operators |
| 3 | DL model execution. For example, in operators |
| 4 | More detailed information than above. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe the simplest way to define the max level of the vlog is corresponding with our namespace.

framework should use VLOG(1) as its highest VLOG level. Since the framework is our core code. VLOG(2) and VLOG(3) could be used in framework for unimportant logs.

operators should use VLOG(3) as its highest VLOG level, since it is the middle level of our framework. operators will invoke platform, memories, eigen, etc.

platform, memory should use VLOG(5) as their highest VLOG level, since they are usually invoked at the end our code.

namespace max vlog level
framework 1
operators 3
memory/platform 5

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

  1. removes doc/.../contribute_to_paddle_cn.md.

cn ->en ?

  1. After this PR, should we update the contribute_to_paddle_cn.md?
  2. In contribute_to_paddle_cn, 提交代码的一些约定 is not in contribute_to_paddle_en.md now. Should this PR add these?

CONTRIBUTING.md Outdated

1. Build and test

Users can build PaddlePaddle natively on Linux and Mac OS X. But to unify the building environment and to make it easy for debugging, the recommended way is [using Docker](https://github.com/PaddlePaddle/Paddle/blob/develop/doc/howto/dev/contribute_to_paddle_en.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the URL of using Docker correct?

Copy link
Collaborator Author

@wangkuiyi wangkuiyi Oct 29, 2017

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is not. Thanks for pointing out. I am correcting it.

clang-formater.......................................(no files to check)Skipped
[my-cool-stuff c703c041] add test file
1 file changed, 0 insertions(+), 0 deletions(-)
create mode 100644 233
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remain the result of git commit? Maybe this result is useful for new developers since they can confirm their pre-commit run successfully.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK. Done.

- Make sure the compiler option `WITH_STYLE_CHECK` is on and the compiler
passes the code style check.
- All code must have unit test.
- Pass all unit tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remain these Code Requirements?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea. I moving it after the Workflow section, as the Code Standard section.

Copy link
Collaborator Author

@wangkuiyi wangkuiyi left a comment

Choose a reason for hiding this comment

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

I removed the Chinese version of this document because I noticed that we haven't been able to keep the Chinese and English version consistent with each other.

In contribute_to_paddle_cn, 提交代码的一些约定 is not in contribute_to_paddle_en.md now. Should this PR add these?

Actually, I translated this part but scattered the information around various parts of the new English version.

- Make sure the compiler option `WITH_STYLE_CHECK` is on and the compiler
passes the code style check.
- All code must have unit test.
- Pass all unit tests.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea. I moving it after the Workflow section, as the Code Standard section.

CONTRIBUTING.md Outdated

1. Build and test

Users can build PaddlePaddle natively on Linux and Mac OS X. But to unify the building environment and to make it easy for debugging, the recommended way is [using Docker](https://github.com/PaddlePaddle/Paddle/blob/develop/doc/howto/dev/contribute_to_paddle_en.md).
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is not. Thanks for pointing out. I am correcting it.

clang-formater.......................................(no files to check)Skipped
[my-cool-stuff c703c041] add test file
1 file changed, 0 insertions(+), 0 deletions(-)
create mode 100644 233
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK. Done.

Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

LGTM

@luotao1
Copy link
Contributor

luotao1 commented Oct 30, 2017

  1. If we put contributed.md out of doc/, we can't show it in current website. Shall we have a special link for this contributed.md ? @jetfuel

  2. I removed the Chinese version of this document

    This PR doesn't remove the Chinese version, should we update the Chinese version later?

  3. We should remove the corresponding links in index_cn.rst and index_en.rst.

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.

4 participants