Skip to content

Commit

Permalink
[docs][ci] Add CI reproducability docs (apache#10912)
Browse files Browse the repository at this point in the history
This adds info about `ci.py` to the docs and also re-arranges things a bit to consolidate/de-duplicate information

Co-authored-by: driazati <[email protected]>
  • Loading branch information
driazati and driazati authored Apr 6, 2022
1 parent 591a000 commit 9bd19bb
Show file tree
Hide file tree
Showing 11 changed files with 121 additions and 74 deletions.
8 changes: 4 additions & 4 deletions docs/contribute/ci.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
Using TVM's CI
==============

.. contents::
:local:

TVM uses Jenkins for running Linux continuous integration (CI) tests on
`branches <https://ci.tlcpack.ai/job/tvm/>`_ and
`pull requests <https://ci.tlcpack.ai/job/tvm/view/change-requests/>`_ through a
Expand Down Expand Up @@ -58,10 +61,7 @@ the failing job to view the logs. Note:
Reproduce Failures
------------------

Most TVM Python tests run under |pytest|_ and
can be run as described in :ref:`pr-testing`. For a closer environment to the one
than runs in CI you can run the docker images directly, build TVM, and execute
tests inside the container. See :ref:`docker_images` for details.
Most TVM Python tests run under |pytest|_ and can be run as described in :ref:`pr-testing`.

Keeping CI Green
****************
Expand Down
16 changes: 12 additions & 4 deletions docs/contribute/code_guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
Code Guide and Tips
===================

.. contents::
:depth: 2
:local:

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.

Expand All @@ -34,14 +38,18 @@ C++ Code Styles
pass by value is better than pass by const reference in such cases.
- Favor ``const`` member function when possible.

We use `clang-format` to enforce the code style. Because different version
We use ``clang-format`` to enforce the code style. Because different version
of clang-format might change by its version, it is recommended to use the same
version of the clang-format as the main one.
You can also use the following command via docker.

.. code:: bash
docker/bash.sh tlcpack/ci-lint clang-format-10 [path-to-file]
# Run a specific file through clang-format
docker/bash.sh ci_lint clang-format-10 [path-to-file]
# Run all linters, including clang-format
python tests/scripts/ci.py lint
clang-format is also not perfect, when necessary, you can use disble clang-format on certain code regions.
Expand Down Expand Up @@ -78,8 +86,8 @@ Because clang-format may not recognize macros, it is recommended to use macro li
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``
- Stick to language features as in ``python 3.6``
- Check your code style using ``python tests/scripts/ci.py lint``
- Stick to language features in ``python 3.7``


Writing Python Tests
Expand Down
21 changes: 6 additions & 15 deletions docs/contribute/code_review.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@
.. _code_review_guide:


Perform Code Reviews
====================
Code Reviews
============

.. contents::
:depth: 2
:local:

Open source code is maintained by a community with diverse backgrounds, interests, and goals.
Hence it is important to provide clear, documented and maintainable code and processes. Code reviews are a
Expand Down Expand Up @@ -152,18 +155,6 @@ Our goal is to strive to be consistent and objective but all of us are unfortuna
Additional Recommendations
--------------------------

Scope the PRs
~~~~~~~~~~~~~

We recommend authors to send well scoped PRs that are easy to review and revert in case there is a problem.
Authors avoid merging multiple unrelated changes into a single PR and split them into separate PRs.

Label the PRs with Area Prefix
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
When sending pull requests, it is helpful to prefix the PR title with the areas related PR(e.g. use [TIR] for TIR-related changes).
This would help people recognize the related areas and find PRs they are interested in.


Deliberate on API and Data Structures
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A 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, argument definitions and behavior.
Expand Down Expand Up @@ -193,7 +184,7 @@ Minimize Dependencies
~~~~~~~~~~~~~~~~~~~~~
Always be cautious in introducing dependencies. While it is important to reuse code and avoid reinventing the wheel,
dependencies can increase burden of users in deployment. A good design principle is that a feature or function
should only have a dependecy if/when a user actually use it.
should only have a dependency if/when a user actually use it.

Concise Implementation
~~~~~~~~~~~~~~~~~~~~~~
Expand Down
5 changes: 5 additions & 0 deletions docs/contribute/committer_guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@

Committer Guide
===============

.. contents::
:depth: 2
:local:

This is an evolving document to provide some helpful tips for committers.
Most of them are lessons learned during development.
We welcome every committer to contribute to this document.
Expand Down
5 changes: 4 additions & 1 deletion docs/contribute/community.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@
TVM Community Guidelines
========================

TVM adopts the Apache style model and governs by merit. We believe that it is important to create an inclusive community where everyone can use, contribute to, and influence the direction of the project. See `CONTRIBUTORS.md <https://github.com/apache/tvm/blob/main/CONTRIBUTORS.md>`_ for the current list of contributors.
.. contents::
:depth: 2
:local:


TVM adopts the Apache style model and governs by merit. We believe that it is important to create an inclusive community where everyone can use, contribute to, and influence the direction of the project. See `CONTRIBUTORS.md <https://github.com/apache/tvm/blob/main/CONTRIBUTORS.md>`_ for the current list of contributors.

General Development Process
---------------------------
Expand Down
8 changes: 6 additions & 2 deletions docs/contribute/document.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@
.. _doc_guide:

Write Documentation for TVM
===========================
Documentation
=============

.. contents::
:depth: 2
:local:

TVM documentation loosely follows the `formal documentation style described by
Divio <https://documentation.divio.com>`_. This system has been chosen because
Expand Down
5 changes: 5 additions & 0 deletions docs/contribute/error_handling.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@

Error Handling Guide
====================

.. contents::
:depth: 2
:local:

TVM contains structured error classes to indicate specific types of error.
Please raise a specific error type when possible, so that users can
write code to handle a specific error category if necessary.
Expand Down
4 changes: 4 additions & 0 deletions docs/contribute/git_howto.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
Git Usage Tips
==============

.. contents::
:depth: 2
:local:

Here are some tips for git workflow.

How to resolve a conflict with ``main``
Expand Down
6 changes: 3 additions & 3 deletions docs/contribute/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ Here are guidelines for contributing to various aspect of the project:
:maxdepth: 2

community
pull_request
code_review
committer_guide
document
code_guide
error_handling
pull_request
git_howto
ci
release_process
release_process
error_handling
109 changes: 66 additions & 43 deletions docs/contribute/pull_request.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,73 +18,65 @@
Submit a Pull Request
=====================

This is a quick guide to submit a pull request, please also refer to the detailed guidelines.
.. contents::
:depth: 2
:local:

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

- We recommend authors send well scoped PRs that are easy to review and revert in case there is a problem. As such, authors should avoid merging multiple unrelated changes into a single PR
- Before you submit a PR, please rebase your code on the most recent version of ``main``, you can do it by
running

.. code:: bash
git remote add upstream [url to tvm repo]
git fetch upstream
git rebase upstream/main
- Make sure code style check pass by typing the following command, and all the existing test-cases pass.
- Make sure code passes lint checks

.. code:: bash
.. code:: bash
# Run all lint steps.
docker/lint.sh
# While the lint commands used should be identical to those run in CI, this command reproduces
# the CI lint procedure exactly (typically helpful for debugging lint script errors or
# to avoid installing tools manually)
python tests/scripts/ci.py lint
# To run steps individually, specify their step names on the command-line. An incorrectly
# spelled step name causes the tool to print all available steps.
docker/lint.sh <step_name> ...
# Run all lint steps.
docker/lint.sh
# While the lint commands used should be identical to those run in CI, this command reproduces
# the CI lint procedure exactly (typically helpful for debugging lint script errors).
docker/bash.sh ci_lint ./tests/scripts/task_lint.sh
# To run steps individually, specify their step names on the command-line. An incorrectly
# spelled step name causes the tool to print all available steps.
docker/lint.sh <step_name> ...
When the clang-format lint check fails, run git-clang-format as follows to automatically reformat
your code:
If the clang-format lint check fails, run git-clang-format as follows to automatically reformat
your code:

.. code:: bash
.. code:: bash
# Run clang-format check for all the files that changed since upstream/main
docker/bash.sh ci_lint ./tests/lint/git-clang-format.sh upstream/main
# Run clang-format check for all the files that changed since upstream/main
docker/bash.sh ci_lint ./tests/lint/git-clang-format.sh upstream/main
- 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 and fix the problems reported by automatic checks.
- Request code reviews from other contributors and improves your patch according to feedbacks.
- `Create a pull request <https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request>`_ and fix the problems reported by CI checks.
- Request code reviews from other contributors and improve your patch according to their reviews by ``@``-ing them in your pull request. Tags in PR titles will automatically tag subscribed users, so make sure to put relevant topics in your PR titles (e.g. ``[microTVM] a cool change`` and not ``a cool change for microTVM``).

- 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.


- The PR can be merged after the reviewers approve the pull request.

CI Environment
--------------
We use docker container to create stable CI environments
that can be deployed to multiple machines.
Because we want a relatively stable CI environment and make use of pre-cached image,
all of the CI images are built and maintained by committers.

Upgrade of CI images can cause problems and need fixes to accommodate the new env.
Here is the protocol to update CI image:

- Send PR to upgrade build script in the repo
- Can be done by a contributor, the following steps need committership.
- Build the new docker image
- Tag the docker image with a new version and push to tvmai
- Update the version(most of the time increase the minor version) in the Jenkinsfile, send a PR.
- Fix any issues wrt to the new image versions in the PR.
- Merge the PR and now we are in new version.
- Tag the new version as the latest.
- Periodically cleanup the old versions on local workers
We use Docker images to create stable CI environments that can be deployed to multiple machines.
Follow the steps in `this issue template <https://github.com/apache/tvm/issues/new?assignees=&labels=&template=ci-image.md&title=%5BCI+Image%5D+>`_
to update a CI Docker image.

.. _pr-testing:

Expand All @@ -93,20 +85,51 @@ Testing
Even though we have hooks to run unit tests automatically for each pull request, it's always recommended to run unit tests
locally beforehand to reduce reviewers' burden and speedup review process.

Docker (recommended)
^^^^^^^^^^^^^^^^^^^^
``tests/scripts/ci.py`` replicates the CI environment locally and provides a user-friendly interface.
The same Docker images and scripts used in CI are used directly to run tests. It also deposits builds
in different folders so you can maintain multiple test environments without rebuilding from scratch
each time (e.g. you can test a change in CPU and i386 while retaining incremental rebuilds).

.. code:: bash
# see all available platforms
python tests/scripts/ci.py --help
python tests/scripts/ci.py cpu --help
# run the CPU build in the ci_cpu docker container (build will be left in
# the build-cpu/ folder)
# note: the CPU and GPU Docker images are quite large and may take some
# time to download on their first use
python tests/scripts/ci.py cpu
# run the CPU build in the ci_cpu docker container and then run unittests
python tests/scripts/ci.py cpu --unittest
# quickly iterate by running a specific test and skipping the rebuild each time
python tests/scripts/ci.py cpu --skip-build --tests tests/python/unittest/test_tir_transform_inject_rolling_buffer.py::test_upscale
# run the CPU build and drop into a shell in the container
python tests/scripts/ci.py cpu --interactive
C++ (local)
^^^^^^^^^^^

Running the C++ tests requires installation of gtest, following the instructions in
:ref:`install-from-source-cpp-tests`

C++
^^^

.. code:: bash
# assume you are in tvm source root
TVM_ROOT=`pwd`
./tests/scripts/task_cpp_unittest.sh
Python
^^^^^^
Python (local)
^^^^^^^^^^^^^^
Necessary dependencies:

.. code:: bash
Expand Down
8 changes: 6 additions & 2 deletions docs/contribute/release_process.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@
.. _release_process:

Apache TVM Release Process
==========================
Release Process
===============

.. contents::
:depth: 2
:local:

The release manager role in TVM means you are responsible for a few different things:

Expand Down

0 comments on commit 9bd19bb

Please sign in to comment.