Skip to content

Commit

Permalink
[DOCS] Detailed contributor guide, doc refactor (apache#1220)
Browse files Browse the repository at this point in the history
  • Loading branch information
tqchen authored Jun 1, 2018
1 parent 9c299a9 commit f9965c1
Show file tree
Hide file tree
Showing 24 changed files with 478 additions and 295 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ list of contributions that are welcomed

How to Contribute
-----------------
See [Contributor guide](docs/how_to/contribute.md) on tips for contributions.
See [Contributor guide](http://tvm.ai/contribute/) on tips for contributions.


Committers
Expand Down
4 changes: 0 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@
[![GitHub license](http://dmlc.github.io/img/apache2.svg)](./LICENSE)
[![Build Status](http://mode-gpu.cs.washington.edu:8080/buildStatus/icon?job=dmlc/tvm/master)](http://mode-gpu.cs.washington.edu:8080/job/dmlc/job/tvm/job/master/)

[Installation](docs/how_to/install.md) |
[Documentation](http://docs.tvm.ai) |
[Tutorials](http://tutorials.tvm.ai) |
[Operator Inventory](topi) |
[FAQ](docs/faq.md) |
[Contributors](CONTRIBUTORS.md) |
[Community](http://tvm.ai/community.html) |
[Release Notes](NEWS.md)
Expand Down
10 changes: 5 additions & 5 deletions apps/android_deploy/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ Here's a piece of example for `config.mk`.

```makefile
APP_ABI = arm64-v8a

APP_PLATFORM = android-17

# whether enable OpenCL during compile
USE_OPENCL = 1

# the additional include headers you want to add, e.g., SDK_PATH/adrenosdk/Development/Inc
ADD_C_INCLUDES = /opt/adrenosdk-osx/Development/Inc

# the additional link libs you want to add, e.g., ANDROID_LIB_PATH/libOpenCL.so
ADD_LDLIBS = libOpenCL.so
```
Expand All @@ -99,7 +99,7 @@ If everything goes well, you will find compile tools in `/opt/android-toolchain-

### Place compiled model on Android application assets folder

Follow instruction to get compiled version model for android target [here.](https://github.com/dmlc/tvm/blob/master/docs/how_to/deploy_android.md#build-model-for-android-target)
Follow instruction to get compiled version model for android target [here.](https://tvm.ai/deploy/android.html)

Copied these compiled model deploy_lib.so, deploy_graph.json and deploy_param.params to apps/android_deploy/app/src/main/assets/ and modify TVM flavor changes on [java](https://github.com/dmlc/tvm/blob/master/apps/android_deploy/app/src/main/java/ml/dmlc/tvm/android/demo/MainActivity.java#L81)

Expand Down
2 changes: 1 addition & 1 deletion apps/howto_deploy/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ Type the following command to run the sample code under the current folder(need
./run_example.sh
```

Checkout [How to Deploy TVM Modules](http://docs.tvm.ai/how_to/deploy.html) for more information.
Checkout [How to Deploy TVM Modules](http://docs.tvm.ai/deploy/cpp_deploy.html) for more information.
4 changes: 2 additions & 2 deletions docs/api_links.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Links to C++/JS API References
==============================
Links to C++ and JS API References
==================================

This page contains links to API references that are build with different doc build system.

Expand Down
39 changes: 39 additions & 0 deletions docs/contribute/code_guide.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
.. _code_guide:

Code Guide and Tips
===================

This is a document used to record tips in tvm codebase for reviewers and contributors.
Most of them are summarized through lessons during the contributing and process.


C++ Code Styles
---------------
- Use the Google C/C++ style.
- The public facing functions are documented in doxygen format.
- Favor concrete type declaration over ``auto`` as long as it is short.
- Favor passing by const reference (e.g. ``const Expr&``) over passing by value.
Except when the function consumes the value by copy constructor or move,
pass by value is better than pass by const reference in such cases.

Python Code Styles
------------------
- The functions and classes are documented in `numpydoc <https://numpydoc.readthedocs.io/en/latest/>`_ format.
- Check your code style using ``make pylint``


Handle Integer Constant Expression
----------------------------------
We often need to handle constant integer expressions in tvm. Before we do so, the first question we want to ask is that is it really necessary to get a constant integer. If symbolic expression also works and let the logic flow, we should use symbolic expression as much as possible. So the generated code works for shapes that are not known ahead of time.

Note that in some cases we cannot know certain information, e.g. sign of symbolic variable, it is ok to make assumptions in certain cases. While adding precise support if the variable is constant.

If we do have to get constant integer expression, we should get the constant value using type ``int64_t`` instead of ``int``, to avoid potential integer overflow. We can always reconstruct an integer with the corresponding expression type via ``make_const``. The following code gives an example.

.. code:: c++

Expr CalculateExpr(Expr value) {
int64_t int_value = GetConstInt<int64_t>(value);
int_value = CalculateExprInInt64(int_value);
return make_const(value.type(), int_value);
}
47 changes: 47 additions & 0 deletions docs/contribute/code_review.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
Perform Code Reviews
====================

This is a general guideline for code reviewers. First of all, while it is great to add new features to a project, we must also be aware that each line of code we introduce also brings **technical debt** that we may have to eventually pay.

Open source code is maintained by a community with diverse backend, and it is even more important to bring clear, documented and maintainable code. Code reviews are shepherding process to spot potential problems, improve quality of the code. We should, however, not rely on code review process to get the code into a ready state. Contributors are encouraged to polish the code to a ready state before requesting reviews. This is especially expected for code owner and comitter candidates.

Here are some checklists for code reviews, it is also helpful reference for contributors

Hold the Highest Standard
-------------------------
The first rule for code reviewers is to always keep the highest standard, and do not approve code just to "be friendly". Good, informative critics each other learn and prevents technical debt in early stages.

Ensure Test Coverage
--------------------
Each new change of features should introduce test cases, bug fixes should include regression tests that prevent the problem from happening again.

Documentations are Mandatory
----------------------------
Documentation is usually a place we overlooked, new functions or change to a function should be directly updated in documents. A new feature is meaningless without documentation to make it accessible. See more at :ref:`doc_guide`

Deliberate on User-facing API
-----------------------------
A good, minimum and stable API is critical to the project’s life. A good API makes a huge difference. Always think very carefully about all the aspects including naming, arguments definitions and behavior. One good rule to check is to be consistent with existing well-known package’s APIs if the feature overlap. For example, tensor operation APIs should always be consistent with the numpy.

Minimum Dependency
------------------
Always be cautious in introducing dependencies. While it is important to reuse code and not reinventing the wheel, dependencies can increase burden of users in deployment. A good design principle only depends on the part when a user actually use it.

Ensure Readability
------------------
While it is hard to implement a new feature, it is even harder to make others understand and maintain the code you wrote. It is common for a PMC or committer to not being able to understand certain contributions. In such case, a reviewer should say "I don’t understand" and ask the contributor to clarify. We highly encourage code comments which explain the code logic along with the code.

Concise Implementation
----------------------
Some basic principles applied here: favor vectorized array code over loops, is there existing API that solves the problem.

Document Lessons in Code Reviews
--------------------------------
When you find there are some common lessons that can be summarized in the guideline,
add it to the :ref:`code_guide`.
It is always good to refer to the guideline document when requesting changes,
so the lessons can be shared to all the community.

Respect each other
------------------
The code reviewers and contributors are paying the most precious currencies in the world -- time. We are volunteers in the community to spend the time to build good code, help each other, learn and have fun hacking.
88 changes: 88 additions & 0 deletions docs/contribute/document.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
.. _doc_guide:

Write Document and Tutorials
============================

We use the `Sphinx <http://sphinx-doc.org>`_ for the main documentation.
Sphinx support both the reStructuredText and markdown.
When possible, we encourage to use reStructuredText as it has richer features.
Note that the python doc-string and tutorials allow you to embed reStructuredText syntax.


Document Python
---------------
We use `numpydoc <https://numpydoc.readthedocs.io/en/latest/>`_
format to document the function and classes.
The following snippet gives an example docstring.
We always document all the public functions,
when necessary, provide an usage example of the features we support(as shown below).

.. code:: python
def myfunction(arg1, arg2, arg3=3):
"""Briefly describe my function.
Parameters
----------
arg1 : Type1
Description of arg1
arg2 : Type2
Description of arg2
arg3 : Type3, optional
Description of arg3
Returns
-------
rv1 : RType1
Description of return type one
Examples
--------
.. code:: python
# Example usage of myfunction
x = myfunction(1, 2)
"""
return rv1
Be careful to leave blank lines between sections of your documents.
In the above case, there has to be a blank line before `Parameters`, `Returns` and `Examples`
in order for the doc to be built correctly. To add a new function to the doc,
we need to add the `sphinx.autodoc <http://www.sphinx-doc.org/en/master/ext/autodoc.html>`_
rules to the `docs/api/python <https://github.com/dmlc/tvm/tree/master/docs/api/python>`_).
You can refer to the existing files under this folder on how to add the functions.


Document C++
------------
We use the doxgen format to document c++ functions.
The following snippet shows an example of c++ docstring.

.. code:: c++

/*!
* \brief Description of my function
* \param arg1 Description of arg1
* \param arg2 Descroption of arg2
* \returns describe return value
*/
int myfunction(int arg1, int arg2) {
// When necessary, also add comment to clarify internal logics
}
Besides documenting function usages, we also highly recommend contributors
to add comments about code logics to improve readability.


Write Tutorials
---------------
We use the `sphinx-gallery <https://sphinx-gallery.github.io/>`_ to build python tutorials.
You can find the source code under `tutorials <https://github.com/dmlc/tvm/tree/master/tutorials>`_ quite self explanatory.
One thing that worth noting is that the comment blocks are written in reStructuredText instead of markdown so be aware of the syntax.

The tutorial code will run on our build server to generate the document page.
So we may have a restriction like not being able to access a remote Raspberry Pi,
in such case add a flag variable to the tutorial (e.g. `use_rasp`) and allow users to easily switch to the real device by changing one flag.
Then use the existing environment to demonstrate the usage.
57 changes: 57 additions & 0 deletions docs/contribute/git_howto.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Git Usage Tips

Here are some tips for git workflow.

## How to resolve conflict with master
- First rebase to most recent master
```bash
# The first two steps can be skipped after you do it once.
git remote add upstream [url to tvm repo]
git fetch upstream
git rebase upstream/master
```
- The git may show some conflicts it cannot merge, say ```conflicted.py```.
- Manually modify the file to resolve the conflict.
- After you resolved the conflict, mark it as resolved by
```bash
git add conflicted.py
```
- Then you can continue rebase by
```bash
git rebase --continue
```
- Finally push to your fork, you may need to force push here.
```bash
git push --force
```

## How to combine multiple commits into one
Sometimes we want to combine multiple commits, especially when later commits are only fixes to previous ones,
to create a PR with set of meaningful commits. You can do it by following steps.
- Before doing so, configure the default editor of git if you haven't done so before.
```bash
git config core.editor the-editor-you-like
```
- Assume we want to merge last 3 commits, type the following commands
```bash
git rebase -i HEAD~3
```
- It will pop up an text editor. Set the first commit as ```pick```, and change later ones to ```squash```.
- After you saved the file, it will pop up another text editor to ask you modify the combined commit message.
- Push the changes to your fork, you need to force push.
```bash
git push --force
```

## Reset to the most recent master
You can always use git reset to reset your version to the most recent master.
Note that all your ***local changes will get lost***.
So only do it when you do not have local changes or when your pull request just get merged.
```bash
git reset --hard [hash tag of master]
git push --force
```

## What is the consequence of force push
The previous two tips requires force push, this is because we altered the path of the commits.
It is fine to force push to your own fork, as long as the commits changed are only yours.
30 changes: 30 additions & 0 deletions docs/contribute/index.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
Contribute to TVM
=================

TVM has been developed by community members.
Everyone is welcomed to contribute.
We value all forms of contributions, including, but not limited to:

- Code reviewing of the existing patches.
- Documentation and usage examples
- Community participation in forums and issues.
- Code readability and developer guide

- We welcome contributions that add code comments
to improve readability
- We also welcome contributions to docs to explain the
design choices of the internal.

- Test cases to make the codebase more robust
- Tutorials, blog posts, talks that promote the project.

Here are guidelines for contributing to various aspect of the project:

.. toctree::
:maxdepth: 2

code_review
document
code_guide
pull_request
git_howto
26 changes: 26 additions & 0 deletions docs/contribute/pull_request.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
Submit a Pull Request
=====================

This is a quick guide to submit a pull request, please also refer to the detailed guidelines.

- Before submit, please rebase your code on the most recent version of master, you can do it by

.. code:: bash
git remote add upstream [url to tvm repo]
git fetch upstream
git rebase upstream/master
- Make sure code style check pass by typing ``make lint``, and all the existing test-cases pass.
- Add test-cases to cover the new features or bugfix the patch introduces.
- Document the code you wrote, see more at :ref:`doc_guide`
- Send the pull request, fix the problems reported by automatic checks.
Request code reviews from other contributors and improves your patch according to feedbacks.

- To get your code reviewed quickly, we encourage you to help review others' code so they can do the favor in return.
- Code review is a shepherding process that helps to improve contributor's code quality.
We should treat it proactively, to improve the code as much as possible before the review.
We highly value patches that can get in without extensive reviews.
- The detailed guidelines and summarizes useful lessons.

- The patch can be merged after the reviewers approve the pull request.
25 changes: 25 additions & 0 deletions docs/deploy/android.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Deploy to Android


## Build model for Android Target

NNVM compilation of model for android target could follow same approach like android_rpc.

An reference exampe can be found at [chainer-nnvm-example](https://github.com/tkat0/chainer-nnvm-example)

Above example will directly run the compiled model on RPC target. Below modification at [rum_mobile.py](https://github.com/tkat0/chainer-nnvm-example/blob/5b97fd4d41aa4dde4b0aceb0be311054fb5de451/run_mobile.py#L64) will save the compilation output which is required on android target.

```
lib.export_library("deploy_lib.so", ndk.create_shared)
with open("deploy_graph.json", "w") as fo:
fo.write(graph.json())
with open("deploy_param.params", "wb") as fo:
fo.write(nnvm.compiler.save_param_dict(params))
```

deploy_lib.so, deploy_graph.json, deploy_param.params will go to android target.

## TVM Runtime for Android Target

Refer [here](https://github.com/dmlc/tvm/blob/master/apps/android_deploy/README.md#build-and-installation) to build CPU/OpenCL version flavor TVM runtime for android target.
From android java TVM API to load model & execute can be refered at this [java](https://github.com/dmlc/tvm/blob/master/apps/android_deploy/app/src/main/java/ml/dmlc/tvm/android/demo/MainActivity.java) sample source.
Loading

0 comments on commit f9965c1

Please sign in to comment.