docs: Contribution guidelines (#1525)

This updates a number of docs pages to infrastructure and workflow topics.
acts-project · Sep 19, 2022 · d9816f2 · d9816f2
1 parent 3da30d6
commit d9816f2
Show file tree

Hide file tree

Showing 14 changed files with 225 additions and 24 deletions.
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
@@ -195,10 +195,8 @@ integrate clang-format with your favourite IDE (e.g.
 `eclipse <https://marketplace.eclipse.org/content/cppstyle>`_,
 `Xcode <https://github.com/travisjeffery/ClangFormat-Xcode>`_,
 `emacs <http://clang.llvm.org/docs/ClangFormat.html#emacs-integration>`_).
-The Acts CI system will automatically apply code reformatting using the
-provided clang-format configuration once pull requests are opened.
-However, developers are encouraged to use this code formatter also
-locally to reduce conflicts due to formatting issues.
+The Acts CI system will automatically check code formatting using the
+provided clang-format configuration and will notify incompatible formatting.
 
 In addition, some conventions are used in Acts code, details can be
 found `here <https://acts.readthedocs.io/en/latest/codeguide.html>`_.

diff --git a/docs/codeguide.md b/docs/codeguide.md
@@ -209,4 +209,4 @@ Files that contain only symbols in the `detail` namespace should be moved in a`d
 
 ## Formatting
 
-Code formatting is handled by [ClangFormat](https://clang.llvm.org/docs/ClangFormat.html). A configuration file is available in the repository root. Please set up your editor/IDE to format files automatically.
+See [](howto_format)
diff --git a/docs/contribution/clang_tidy.md b/docs/contribution/clang_tidy.md
@@ -1 +1,38 @@
 # What is clang-tidy?
+
+[`clang-tidy`](https://clang.llvm.org/extra/clang-tidy/) is a static analysis
+tool using the LLVM / clang tooling. It can detect a number of issues with C++
+code, like common readability problems, performance or memory safety issues.
+
+The ACTS CI automatically runs `clang-tidy` on all compilation units to detect
+as many problems as possible, and requires developers to resolve them. It is
+configured with a file `.clang-tidy` located in the repository root. Many
+editors / IDEs support `clang-tidy` and will automatically pick up this
+configuration file.
+
+By default, only a limited number of checks are configured in this file, but
+this list might change in the future. If you experience CI failures that are
+associated with `clang-tidy`, you can drill down into the CI job outputs to get
+a report on the issues it detected. The report should give you an error /
+warning code, e.g. `readability-braces-around-statements`. The LLVM
+documentation has details on all possible error codes, in this particular
+example you would find it [here][readability].  This page will tell you that
+`clang-tidy` wants you to replace 
+
+```cpp
+if (condition)
+   statement;
+```
+
+with
+
+```cpp
+if (condition) {
+   statement;
+}
+```
+
+Some error codes are less obvious, or not this trivial to fix. When in doubt,
+@mention the ACTS maintainers on your pull request.
+
+[readability]: https://clang.llvm.org/extra/clang-tidy/checks/readability/braces-around-statements.html
diff --git a/docs/contribution/documentation_build.md b/docs/contribution/documentation_build.md
@@ -1 +1,3 @@
 # How do I build the documentation?
+
+See [](build_docs)
diff --git a/docs/contribution/physmon.md b/docs/contribution/physmon.md
@@ -1 +1,95 @@
 # What is physmon?
+
+The ACTS CI runs a suite of physics performance monitoring jobs dubbed
+*physmon*. The purpose is to monitor and detect changes in the physics
+performance, both intentional and accidental.
+
+The associated job will run a number of workflow combinations. Currently this
+includes the truth tracking and OpenDataDetector *full chain* workflows. The
+latter is further split into configurations with full seeding, truth smeared or
+truth estimated seeds. These jobs produce performance output files.
+
+These performance output files are then evaluated using the programs dubbed
+[](analysis_apps). After this step, the job will then run comparisons of the
+diagnostics histograms created by these programs. If the comparisons indicate
+that the histogram contents changed meaningfully, the job will fail and report
+this on a pull request.
+
+## How do I investigate a physmon failure?
+
+The physmon CI job attaches its results as an artifact to the CI run (also for successful runs)
+From your pull request, you need to click on the *Checks* tab at the top:
+
+:::{figure} ../figures/physmon/physmon_checks.png
+:width: 150px
+:align: center
+:::
+
+From there, click on the *Builds* workflow on the left:
+
+:::{figure} ../figures/physmon/physmon_run.png
+:width: 300px
+:align: center
+:::
+
+On the workflow overview, scroll down to find the attached artifacts, and locate the *physmon* artifact. You can click to download it:
+
+:::{figure} ../figures/physmon/physmon_artifact.png
+:width: 100%
+:align: center
+:::
+
+:::{note}
+GitHub Actions artifacts are deleted after a fixed amount of time. If the CI
+completed a while ago, it is possible this report is no longer available.
+:::
+
+After the download, you need to unzip the archive, whose contents will look similar to this:
+
+```
+acts_analysis_residuals_and_pulls.root
+ckf_seeded.html
+ckf_seeded_plots
+ckf_truth_estimated.html
+ckf_truth_estimated_plots
+ckf_truth_smeared.html
+ckf_truth_smeared_plots
+ivf_seeded.html
+ivf_seeded_plots
+ivf_truth_estimated.html
+ivf_truth_estimated_plots
+ivf_truth_smeared.html
+ivf_truth_smeared_plots
+performance_ckf_seeded.root
+performance_ckf_truth_estimated.root
+performance_ckf_truth_smeared.root
+performance_truth_tracking.root
+performance_vertexing_seeded.root
+performance_vertexing_seeded_hist.root
+performance_vertexing_truth_estimated.root
+performance_vertexing_truth_estimated_hist.root
+performance_vertexing_truth_smeared.root
+performance_vertexing_truth_smeared_hist.root
+run.log
+truth_tracking.html
+truth_tracking_plots
+```
+
+The `.root` files are the performance output files and corresponding histogram
+files. The physmon job log file is also available. Finally, the `_plots` folder
+contain plots of all the histogram comparisons, and the `.html` files contain
+single-file reports showing the detailed results. An example of an HTML report
+looks like this:
+
+:::{figure} ../figures/physmon/physmon_report.png
+:width: 100%
+:align: center
+:::
+
+If you get a physmon job failure on your pull request, please investigate the
+failing report(s), and try to understand if the change causing the
+discrepancies is expected.
+
+In case the conclusion is that the changes are indeed expected, the reference
+files used to generate the comparisons are located in `CI/physmon/reference/`,
+and can be updated to the output files found in the artifact zip archive.
diff --git a/docs/contribution/root_hash_checks.md b/docs/contribution/root_hash_checks.md
@@ -1 +1,3 @@
-# What is root hash checks?
+# What are ROOT hash checks?
+
+See {ref}`root_hash_checks`
diff --git a/docs/contribution/run_formatting.md b/docs/contribution/run_formatting.md
@@ -1 +1,33 @@
+(howto_format)=
+
 # How do I run the formatting?
+
+Code formatting is handled by
+[`clang-format`](https://clang.llvm.org/docs/ClangFormat.html). A configuration
+file is available in the repository root at `.clang-format` and should be used
+to automatically format the code. Many editors / IDEs support `clang-format`
+and also format-on-save actions.
+
+The ACTS CI system will automatically check code formatting using the provided
+`clang-format` configuration and will notify incompatible formatting.
+
+To simplify this, a script located in `CI/check_format` can be used like:
+
+```console
+$ CI/check_format $SOURCE_DIR
+```
+
+In some cases, different `clang-format` versions will result in slightly
+different outputs. In many cases, this is accepted by the CI. However, it is
+recommended to use the same major version of `clang-format` to perform local
+formatting. If you are comfortable with Docker, you can use the [docker image
+used by the
+CI](https://github.com/acts-project/machines/blob/master/format10/Dockerfile).
+You can use the script located in `CI/check_format_local` similar to the
+description above. Other options to obtain a compatible `clang-format` version
+are to use your package manager (e.g. Ubuntu distributions usually offer a set of
+versions to install), or to use statically linked binaries from
+[here](https://github.com/muttleyxd/clang-tools-static-binaries)[^1].
+
+
+[^1]: This repository is external to the ACTS project, so proceed with caution!
diff --git a/docs/examples/analysis_apps.rst b/docs/examples/analysis_apps.rst
@@ -1,4 +1,6 @@
-ROOT based analysis applications
+.. _analysis_apps:
+
+Analysis applications
 ================================
 
 The ACTS examples come with a certain variety of ROOT based validation and performance writers, 

diff --git a/docs/examples/python_bindings.rst b/docs/examples/python_bindings.rst
@@ -88,13 +88,28 @@ repository. They are located under ``$REPO_ROOT/Examples/Python/tests``, and
 intend to cover the public API of the python bindings. A set of tests also
 executed the standalone example scripts.
 
-To run these python based tests, ``pytest`` and a few other dependencies need to be installed. They can be
-installed via ``pip install -r Examples/Python/tests/requirements.txt`` from the repository root. It is recommended to install these packages
-in `virtual environment`_. You can then simply run ``pytest`` from the
-repository root.
+To run these python based tests, ``pytest`` and a few other dependencies need
+to be installed. They can be installed via ``pip install -r
+Examples/Python/tests/requirements.txt`` from the repository root.  You can
+then simply run ``pytest`` from the repository root.
+
+.. tip::
+
+   It is **strongly recommended** to use a `virtual environment`_ for
+   this purpose! For example, run 
+
+   .. code-block:: console
+
+      $ python -m venv docvenv
+      $ source docvenv/bin/activate
+
+   to create a local virtual environment, and then run the `pip` command above.
+
 
 .. _virtual environment: https://realpython.com/python-virtual-environments-a-primer/
 
+.. _root_hash_checks:
+
 ROOT file hash regression checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

diff --git a/docs/figures/physmon/physmon_artifact.png b/docs/figures/physmon/physmon_artifact.png
diff --git a/docs/figures/physmon/physmon_checks.png b/docs/figures/physmon/physmon_checks.png
diff --git a/docs/figures/physmon/physmon_report.png b/docs/figures/physmon/physmon_report.png
diff --git a/docs/figures/physmon/physmon_run.png b/docs/figures/physmon/physmon_run.png
diff --git a/docs/getting_started.md b/docs/getting_started.md
@@ -183,21 +183,34 @@ However, if you have the necessary prerequisites installed it is possible to use
 it locally. Acts developers regularly use different Linux distributions
 and macOS to build and develop Acts.
 
+(build_docs)=
 ## Building the documentation
 
 The documentation uses [Doxygen][doxygen] to extract the source code
-documentation and [Sphinx][sphinx] with the [Breathe][breathe] and
-[Exhale][exhale] extensions to generate the documentation website. To build the
-documentation locally, you need to have [Doxygen][doxygen] installed from your
-package manager. [Sphinx][sphinx] and its extensions can be installed using the
-Python package manager via
+documentation and [Sphinx][sphinx] with the [Breathe][breathe] extension to
+generate the documentation website. To build the documentation locally, you
+need to have [Doxygen][doxygen] version `1.9.5` or newer installed.
+[Sphinx][sphinx] and a few other depencencies can be installed using the Python
+package manager `pip`:
 
 ```console
 $ cd <source>
-# --user installs to a user-specific directory instead of the system
-$ pip install --user -r docs/requirements.txt
+$ pip install -r docs/requirements.txt
 ```
 
+:::{tip}
+It is **strongly recommended** to use a [virtual
+environment](https://realpython.com/python-virtual-environments-a-primer/) for
+this purpose! For example, run 
+
+```console
+$ python -m venv docvenv
+$ source docvenv/bin/activate
+```
+
+to create a local virtual environment, and then run the `pip` command above.
+:::
+
 To activate the documentation build targets, the `ACTS_BUILD_DOCS` option has to be set
 
 ```console
@@ -213,19 +226,25 @@ $ cmake --build <build> --target docs # default fast option
 $ cmake --build <build> --target docs-with-api # full documentation
 ```
 
-The default option includes the Doxygen, Sphinx, and the Breathe extension, i.e.
-the source code information can be used in the manually written documentation
-but the full API documentation is not generated. The second target builds the
-full documentation using Exhale to automatically generate the API documentation.
-This is equivalent to the public [Read the Docs][rtd_acts] documentation, but
-the build takes around ten minutes to finish.
+The default option includes the Doxygen, Sphinx, and the Breathe extension,
+i.e. the source code information can be used in the manually written
+documentation but the full API documentation is not generated. The second
+target builds the full documentation to automatically generate full API
+listings. This is equivalent to the public [Read the Docs][rtd_acts]
+documentation, but the build takes a while to finish.
 
 [doxygen]: https://doxygen.nl/
 [sphinx]: https://www.sphinx-doc.org
 [breathe]: https://breathe.readthedocs.io
 [exhale]: https://exhale.readthedocs.io
 [rtd_acts]: https://acts.readthedocs.io
 
+A special phony target exists to clean the documentation output files:
+
+```console
+$ cmake --build <build> --target clean-docs
+```
+
 ## Build options
 
 CMake options can be set by adding `-D<OPTION>=<VALUE>` to the configuration