o2p checklist

Checklist for O2Physics users (Dos, Don'ts, Tips)

Communication

• Join the main communication channels. Read the docs.
• When you post, post in the correct channel.
  • Ask for help in the right support channel.
  • Do not discuss in the announcement channel!!!
  • Take longer discussions into private chats to avoid spamming.

Asking for help

• Before you ask someone for help (on Mattermost or in person), make the effort to look for the answer first. Known issues are usually already described or discussed somewhere.

  • Follow instructions given by the error message (if any).
  • Check the troubleshooting page.
  • Check the announcement channel.
  • Check the support channel.
  • For more specific problems, check more specific channels (Hyperloop, tutorial, PWG).

• If you could not find any answer to your question, phrase your question in a way that gives the other person enough relevant information to help you. Read the docs.

Writing code

Read the documentation before you start making changes!

Follow general good coding practices

• Structure code in a way that makes it easy to extend or modify.
• Do not duplicate code unnecessarily.
• Do not reinvent the wheel unnecessarily.
  • Before you start implementing a new algorithm or a data structure, look around and check whether it already exists (Common/Core, O2, C++ STL).
• Test and fail (in the code flow) as early as possible.
  • Add sanity checks in your init functions (to spot pathological configuration, typically with process switches).
  • Throw meaningful error messages close to the problems.
  • Throw a fatal for critical problems that make further running of the code impossible or useless.
• Write code with a collaborative spirit.
  • Factor out common utilities and make them available to others.
  • Follow the existing O2 conventions.
  • Use meaningful names.
  • Document your code.
  • O2Physics is a project of the ALICE collaboration, not a collection of personal analysis tasks, macros, and scripts.
    • We share the code.
      • We benefit from each other's code. Make it easy for others to use it and to contribute to it.
        • Write code that could be used by others if you died tomorrow. ;-)
      • There is no such thing as private or personal code in O2Physics. It's all collaboration code which can be used and modified by any collaboration member. So code quality, readability, and maintainability all matter.
    • We share the resources.
      • If a PR with a problem is merged, it will eventually have to be fixed later. Until then, it has wasted the compilation resources (in CI build checks) and the Grid resources (in trains) at the expense of everybody else. Don't ignore problems reported by the CI checks.
    • We share the results.
      • In the end, all collaboration members are authors of all collaboration papers, so it is in the interest of every collaboration member to catch as many bugs as possible as soon as possible. And that is much easier to do if the code is well readable and comprehensible and if the logs of PR checks are not full of ignored errors.

C++

• Learn C++. Some important features in O2 are:
  • template
  • concept
  • requires
  • constexpr
  • auto
  • const
  • const&
  • smart pointers (std::shared_ptr, std::unique_ptr)
• Use smart pointers instead of raw pointers.
• Use std::array (fixed length) or std::vector (dynamic length) instead of C arrays.
• Prefer STL algorithms instead of raw loops and other custom implementations.
• Avoid expensive copies.
  • Pass objects bigger than their address (64 b) by reference to const (const&) instead of by value.
• Include what you use.
  • Violating this rule in your file breaks the header dependencies not only for your file but also for other files and can break compilation in all of them!
  • Do not leave comment lines between includes. They prevent clang-format from sorting and grouping the includes.
• Avoid using-directives in the global scope in headers. (reported by O2 linter)
• Use const for runtime constants.
• Use constexpr for compile-time constants.
• Use auto to capture return values to avoid being overspecific with types and to avoid unintended implicit casting.
• Use the fully qualified names for names from the std namespace (e.g. std::abs), enumerators, and small or local namespaces. It makes it easier to deduce where the name comes from and helps to reduce name collisions.

O2-specific

• Use declarative O2 features whenever possible.
• Use common O2Physics tools and utilities (e.g. in Common/Core):
  • RecoDecay
  • event mixing
  • metadata helper
  • table helper
  • track utilities
• Limit the usage of ROOT classes and types to the absolute minimum. O2 tasks are C++ structs, not ROOT classes. Always prefer equivalents from the Standard C++ library, O2, or O2Physics.

Little known facts

• Metadata reading requires the input to be provided via the command line option --aod-file. (The aod-file-parent key in the JSON cannot be used instead.)
• Table description is truncated to 15 characters if longer. (That is the description used for output.)
• [CRITICAL] and [ERROR] log messages do not trigger non-zero exit code. Only [FATAL] messages do.
• If you want a non-zero exit code on errors, use --min-failure-level error.
• AliHyperloop does not check for errors or warnings in the log.

Contributing

Read the documentation before you start making changes!

Validate your work locally

Test your changes locally before you share them with others. That includes the following steps.

• You compile the project with your changes successfully and without compilation warnings.
• You run the code with your changes successfully and without warnings or errors.
• You validate the output of the code with your changes.
• If you make further changes, you test again!

Follow the recompilation instructions properly

The only official instructions on installation and recompilation of O2Physics are in the official documentation. Follow them properly!

There are two kinds of environments to use with O2Physics (and other packages built by aliBuild) which are not interchangeable.

• The "run environment" is loaded by alienv enter and is meant for execution.
• The "build environment" is loaded by direnv when you enter the build directory and is meant for compilation (with ninja).

TL;DR: Never build inside an alienv environment (neither with aliBuild nor with ninja)!

If you do, the recompilation will probably fail with: /lib/x86_64-linux-gnu/libstdc++.so.6: version 'GLIBCXX_3.4.32' not found and you will probably need to delete the build directory (see the output of ls -l ~/alice/sw/BUILD/O2Physics-latest) and build with aliBuild again.

Git

• Learn Git. Read the docs.
  • Absolute basics: clone, remote, pull, push, status, diff, add, commit
  • Comfortable daily usage: branch, checkout, reset, rm, mv, stash, difftool, mergetool, merge, rebase, log
  • More advanced usage: switch, restore, revert, fetch, cherry-pick, reflog
• Use Git branches.
• Do not commit in the master branch. Only update it from upstream.
• Commit self-contained changes.
• Use meaningful commit messages.

GitHub

• Understand the terms "remote repository", "fork", "upstream", "origin".
• Use your real name (in Latin alphabet, matching your Mattermost and SAMS name) on your GitHub profile and a meaningful GitHub username (ideally same as your CERN username on Mattermost). (Don't make it a detective story for the reviewers to figure out who made the PR and how to contact them.)
• Understand what a pull request is and how it works. Read the docs.
• Use meaningful PR titles (what changed where).
  • The PR tag prefix is added automatically. Don't add it by hand. You can still extend the default one.
• Use the PR description field to give more details (why the change was needed, links to related code or PRs).
• PRs are being (re)compiled according to the following algorithm:
  • Untested PRs have priority, pick one at random if they exist.
  • Otherwise roll a dice, 70% to pick a random failed PR, 30% to pick a random successful PR.

PR review

For everyone

• Understand how the PR review works. Read the docs.
• Learn how PR updates work and modify your PR as needed.
  • Do not duplicate PRs. (It's duplicating review efforts and usage of testing resources.)
  • Do not change history (amend, squash, rebase) during review. (You'll overwrite the already reviewed content.)
• Read error messages from failing tests and address them appropriately.
• Convert a PR to draft if not ready to merge (work in progress, required checks fail, pending requested changes, requires another pending PR). You will avoid unnecessary (re)compilations which would slow down merging of other PRs in the queue.
• Do not mark someone else's comment as resolved. (It's up to them to assess whether their comment has been addressed, which is hard to do when the comment is collapsed.)

For code owners (approvers)

• Make sure you understand which part of the repository you are supposed to review.
  • If not sure, ask the convener of the group and check that your assignment in CODEOWNERS is correct.
  • Being in CODEOWNERS gives you permissions to approve PRs.
  • Being in CODEOWNERS does not give you write permissions in the repository, needed for editing and merging PRs. To get write permissions, you must accept the email invitation that should be sent to you by the admin. Write permissions apply in the whole repository.
• Make sure your GitHub email notifications are enabled and you are receiving them in your inbox. (Usual problems: wrong email address, flagged as spam.)
• Make sure you know where the official documentation is (general and PWG-specific).
• Read the guidelines in the documentation and apply them in your review.
• Make sure you understand how auto-merge works.
• Do not approve or enable auto-merge before tests are done (and ideally successful).
  • Checks can reveal unexpected issues that might be worth fixing before merging.
  • GitHub merges the PR with the title it had at the time when auto-merge was enabled and any later changes (e.g. tag prefix added automatically by the title checker) are ignored.
• Make sure you understand what the required checks are, how they work, what their failures mean, and how to fix them.
  • Compilation warnings break the compilation only in tests.
  • Formatting can be fixed by merging the automatic PR or with pre-commit hooks before committing.
  • Compilation tests run on PRs opened as "ready for review", not on drafts.
• Verify that the PR title is correct and informative.
• Inspect errors and warnings reported by the checks and request appropriate action.
• Convert a PR to draft if not ready to merge (work in progress, required checks fail, pending requested changes, requires another pending PR). You will avoid unnecessary (re)compilations which would slow down merging of other PRs in the queue.
• If you are intentionally approving a PR which fails some tests, comment in the PR about your reasoning.
• Do not push your branches to the upstream repository!

Testing, linting, profiling, optimising

Use diagnostic tools to test and improve the code. Read the docs.

• Check the code regularly for bugs and improvements.
  • O2 linter
  • clang-tidy
  • cppcheck
  • clangd
• Profile the resource usage of your workflows.
  • Speedscope flame charts using perf for CPU usage.
  • Hyperloop performance graphs for memory usage.
  • Perfetto UI for investigation of compilation time using .ninja_log converted with ninjatracing.