Skip to content
Snippets Groups Projects
Unverified Commit d5c4f244 authored by Brent Huisman's avatar Brent Huisman Committed by GitHub
Browse files

Docs: Add contribution section (#1261)

Set up contribution guides.

Add a contribution guide, as well as 
* Added contribution section.
* Moved content here from the wiki regarding (contribution) processes.

Also added some incomplete / placeholder sections that can be improved later.
* How to write an example
* How to add a test
* Coding style guidelines
parent 92fdf6c5
No related branches found
No related tags found
No related merge requests found
...@@ -8,7 +8,7 @@ There are many ways to contribute: writing tutorials or blog posts, improving th ...@@ -8,7 +8,7 @@ There are many ways to contribute: writing tutorials or blog posts, improving th
documentation, submitting bug reports and feature requests or writing code which can be documentation, submitting bug reports and feature requests or writing code which can be
incorporated into Arbor itself. incorporated into Arbor itself.
[On our wiki](https://github.com/arbor-sim/arbor/wiki/How-to-contribute) we have laid out how you The Arbor documentation has a [Contributing section](https://arbor.readthedocs.io/en/latest/contrib) where we have laid out how you
can go about making your contribution. can go about making your contribution.
## Get in touch ## Get in touch
......
.. _contribcodingstyle
Coding Guidelines
=================
The main development language of Arbor is C++. For Arbor we start with
the community guidelines set out in the `C++ Core
Guidelines <http://isocpp.github.io/CppCoreGuidelines/>`__. These
guidelines are quite generic, and only give loose guidelines for some
important topics like variable naming and indentation.
This wiki will describe the specific extensions and differences to the
C++ Core Guidelines - variable naming. - code formatting (indentation,
placement of curly braces, ``int const&`` vs ``const int&`` etc.) -
rules for topics not covered in the Core Guidelines (e.g. CUDA). -
exceptions to the rules given in the Core Guidelines. - rules for the
CMake build system and directory structure. - rules for external
dependencies (e.g. Boost).
.. TODO::
This page needs revision.
Code organisation
-----------------
Source files naming conventions
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.. TODO:: describe the public/private header and source code organization.
- ``.cpp`` extension for source files
- ``.hpp`` extension for header files
- Names must be lowercase with words separated with underscores.
Namespaces
----------
All code is in ``namespace arb``.
As an example, the ``std::make_unique<>`` function template was not
provided as part of C++11 (it wasn’t introduced until C++14), and we
would like to use it in Arbor. The code sample below shows how
namespaces are declared and formatted in Arbor:
::
namespace arb {
namespace util {
// just because we aren't using C++14, doesn't mean we shouldn't have make_unique
template <typename T, typename... Args>
std::unique_ptr<T> make_unique(Args&&... args) {
return std::unique_ptr<T>(new T(std::forward<Args>(args) ...));
}
} // namespace util
} // namespace arb
In the example above the namespaces are not indented. However,
namespaces should be indented if they are declared in the middle of
code, to make their existance obvious to the person reading the code.
Use an ``impl`` namespace to hide implementation details that should not
be exposed to user space.
You can use ``using`` statements to import individual types or functions
from a namespace - only if it really improves the readability of your
code - only in a function or class scope: don’t pollute namespaces
Formatting statements
---------------------
A lot of the rules here are purely a matter of personal taste, imposed
by the guy who got to set the rules. That said, it follows accepted
practice in the C++ community (if we accept that not everybody has the
same accepted practice), and if followed consistently will make code
easier to understand.
.. code:: c++
template <typename T>
class array {
public:
using value_type = T;
value_type& operator[] (std::size_t i) {
return data_[i];
}
const value_type& operator[] (std::size_t i) const {
return data_[i];
}
std::size_t size() const {
return size_;
}
private:
value_type* data_;
std::size_t size_;
};
// use new lines and indentation to make complex template expressions
// human readable
template <
typename T,
typename = typename // assert that T is a built-in arithmetic type
std::enable_if<
std::is_arithmetic<T>::value
>
>
T sum(const array<T>& in) {
return std::accumulate(in.begin(), in.end(), 0);
}
TODO: When declaring an operator, should we leave a space between the
operator and the following opening parenthesis or should we follow the
convention we use for functions, where we don’t leave a space?
Indentation and whitespace cleanup
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- No tabs, 4 spaces
- Take the extra effort to remove trailing whitespace (at the end of
the lines and the file).
- Respect 80-column limit, but go for longer lines when they make sense
(and make the code clearer)
Variable naming conventions
~~~~~~~~~~~~~~~~~~~~~~~~~~~
All lowercase, words separated by ``_``, but template parameters follow
camel case:
.. code:: c++
template <typename ValueType>
class my_class {
public:
// ...
private:
ValueType val_;
};
Single letter template parameters should be preferred.
TODO: Or should we force single letter parameters aliased by more
meaningful type names inside the class (either public or private
depending on our intent)?
*Avoid* obfuscated names of old C heritage.
Recurring variables naming conventions
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TODO: Some variable names are recurring a lot inside every code. It
would be nice if we could decide on the most common ones.
- ``count`` or ``cnt``
- ``index`` or ``idx``
- ``iter`` or ``it``
- …
Ben says “depends… I would use ``count`` or ``index`` unless the scope
of the variable is very small. Using ``it`` is standard C++ short hand,
but again for fairly limited scope.”
Member variables
~~~~~~~~~~~~~~~~
Private member variables must be suffixed by ``_``, while public member
variables must not.
TODO: Any conventions about ``static`` variables, ``const``\ s or global
``const``\ s?
Member initialisation lists
~~~~~~~~~~~~~~~~~~~~~~~~~~~
Constructors member initialisation lists should be as follows:
.. code:: c++
// everything goes on one line if clear
class my_class {
public:
my_class(int a):
a_(a)
{}
my_class(int a, int b, int c):
a_(a), b_(b) , c_(c)
{}
private:
int a_ = 0;
int b_ = 0;
int c_ = 0;
};
// use one entry per line if multiple lines needed
class my_class {
public:
my_class(int a, int o, int p):
apple_(a),
orange_(o),
pear_(find_pair_type(p))
{}
private:
int apple_;
int orange_;
int pear_;
};
Member functions
~~~~~~~~~~~~~~~~
Make sure to declare ``const`` if it is not changing the object’s state.
Getters and Setters
~~~~~~~~~~~~~~~~~~~
Before filling up a class with getters and setters, consider seriously
if those members are meant actually to be public. If nonetheless getters
and/or setters are needed, don’t use the ``get_`` and ``set_`` prefixes.
.. code:: c++
template <typename T>
class my_class {
public:
// ...
T value() const {
return value_;
}
void value(const T& val) {
// perhaps do something before assigning, otherwise it could be just public
value_ = val;
}
private:
T value_;
};
Declaring references and pointers
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.. code:: c++
// ok
std::string& s = ...;
const std::string& s = ...;
std::string* s = ...;
const std::string* s = ...;
std::string* const s = ...;
// not ok
std::string &s = ...;
const std::string &s = ...;
std::string *s = ...;
const std::string *s = ...;
std::string *const s = ...;
Generally, we follow C++’s convention for references and pointers, as it
is the style used in the C++ standard, and also the recommendation of
the `C++ Core Guidelines
NL.18 <http://isocpp.github.io/CppCoreGuidelines/#nl18-use-c-style-declarator-layout>`__.
Precedence and the C++ language grammar may offer some support the other
convention, but not enough support!
Macros
~~~~~~
Macros are C-ish, so they must be avoided. If not possible, they must be
written in capitals, with words separated by underscores.
Always use ``{}``, even for single statement ``if``, ``for``, etc
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
It makes code clearer, and avoids nasty bugs that occur when
refactoring. It also avoids some errors when merging with git.
::
// ok
for (auto& v: vector) {
// increment the value!
v++;
}
// bad
for (auto& v: vector)
// increment the value!
v++;
don’t put ``{`` on a new line
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Except when indentation of arguments or when doing member initialization
in constructors.
::
// it makes sense to have the { on a new line here for clarity
std::vector<std::string> foo(
std::vector<std::vector<int>>& values,
std::map<int, std::string>& name_table)
{
// do some work
}
leave a space between ``if``, ``for`` etc and following parenthesis
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Accords with `K&R
style <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#Rl-knr>`__,
and makes a visual distinction with function evaluation
::
// ok
for (auto& v: vector) {
v++;
}
// not ok
for(auto& v: vector) {
v++;
}
use ``using`` instead of ``typedef``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
It is easier to read, consistent with ``auto``:
::
// good
using int_container = std::vector<int>;
// bad
typedef std::vector<int> int_container;
and can be used for template aliases
::
template <typename T>
using aligned_container = std::vector<T, my_fancy_aligned_allocator<T>>;
Use scoped enum instead of enum
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
::
// good
enum class ionKind {sodium, calcium};
// bad
enum ionKind {ion_sodium, ion_calcium};
And stick to the naming scheme for all enums of ``xxxKind`` to make it
clear throughout the code whenever an enum is being used, for example:
::
auto i = current(voltage, ionKind::calcium);
Use ``struct`` for POD wrappers
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
But ``class`` if it has any sort of fancy logic associated with it
Memory management
-----------------
use ``unique_ptr``
~~~~~~~~~~~~~~~~~~
Actually, feel free to use naked pointers in your code, but make sure
that you use smart pointers to handle allocation and freeing of memory.
When a developer sees a naked pointer in Arbor they can think “good, I
don’t have to worry about responsibility for freeing that memory”.
Furthermore, if ``unique_ptr`` handles allocation and freeing of memory,
the user doesn’t have to concern themselves with freeing memory ever.
This practice implies that care must be taken to ensure that the
resource managed by a ``unique_ptr`` has to outlive any raw pointers
that are obtained from its ``get()`` member.
while avoiding ``shared_ptr`` whenever possible
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
If you think long and hard, you will probably realise that you actually
want a ``unique_ptr``. Shared pointers have performance overheads, and
are quite easy to misuse. For example by creating circular references
that ironically lead to memory never being freed.
Header files
------------
use pragma once
~~~~~~~~~~~~~~~
Use ``#pragma once`` to guard against including the same header twice.
This might not be completely standard compliant, but it is supported by
every compiler under the sun, and is much cleaner than ``#ifdef``
guards.
don’t rely on headers being included elsewhere
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
For example, if you use ``std::vector<int>`` in a file, make sure to
have ``#include <vector>`` at the top of the source file.
Relying on headers being include elsewhere can lead to portability
problems, for example on OS X you have to ``#include <cmath>`` for some
math functions that are imported via other header files with gcc on
Linux.
Sort headers alphabetically
~~~~~~~~~~~~~~~~~~~~~~~~~~~
To make it easy to search for a header in a long list of includes.
For example:
.. code:: c++
#include <algorithm>
#include <fstream>
#include <map>
#include <queue>
#include <set>
use C++ wrappers for C standard headers
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.. code:: c++
// good
#include <cmath>
#include <cstdio>
// bad
#include <math.h>
#include <stdio.h>
when calling C stdlib functions, use the ``std::``-prefix versions,
e.g., ``std::printf(...)`` instead of ``printf``. Most of the times C++
wrappers just bring into ``std`` the C declarations, but sometimes the
wrappers have more syntactic sugar and call the same internal builtins
that their C counterparts call (for example GCC).
group headers together
~~~~~~~~~~~~~~~~~~~~~~
In the following order
1. C++ standard libary
2. system C headers (POSIX, kernel interfaces etc.)
3. external libraries
4. public arbor headers
5. private arbor headers
For example:
.. code:: c++
// first C++ standard headers
#include <algorithm>
#include <fstream>
#include <map>
// then system C headers
#include <signal.h>
#include <sys/select.h>
// externals
#include <vector/Vector.hpp>
// public arbor headers
#include <arbor/common_types.hpp>
#include <arbor/simulation.hpp>
// private arbor headers (note we use quotes for private project headers).
#include "cell_group.hpp"
#include "util/optional.hpp"
.. _contribdoc:
Documentation
=====================
The source for `the documentation <https://arbor.readthedocs.io>`__ is
found in the ``/doc`` subdirectory. You can add your contribution to the documentation
in the same way you would contribute code, please see the :ref:`contribpr` section.
.. todo::
This page is under construction.
.. _contribdoc-update:
Update policy
-------------
How to we decide if documentation is good? By observing how effective it is used
in practice. If a question on Arbor (regardless of medium) is satisfactorily
resolved (on both sides!) by with a pointer to the (relevant section in the) docs,
the documentation was good. If, on the other hand, explanation was needed, the
documentation was bad.
If you found the documentation to be insufficiently clear or elaborate, you must
consider this a bug and find or file an `issue <https://github.com/arbor-sim/arbor/issues>`__ and if you are able, make a :ref:`pull request <contribpr>`.
.. _contribdoc-namingconventions:
Labels and filename conventions
-------------------------------
Although it is not absolutely essential to do so, we try to keep to the following conventions
for naming files and labels, with the goal of making it easy to construct one from the other
such that you don't have to remember or look anything up. We try to cross-link where we can,
which means we label where we can, which translates to a large number of labels.
Wherever possible, observe:
* file names: avoid underscores as much as possible. E.g. `cpp/cable_cell.rst` -> `cpp/cablecell.rst`.
* page-level labels in each file: the path concatenated without spaces. E.g. `cpp/cablecell.rst` -> `cppcablecell`.
* heading labels in a file: `pagelevel-sectionname`. Feel free to slugify long headings.
E.g. the morphology section in the C++ cable cell docs could be `cppcablecell-morph`.
.. _contribdoc-lang:
Language
--------
Although the primary language of Arbor is C++, we use English for most of the non-code.
If in doubt, we recommend following the European Union's
`English style guide <https://ec.europa.eu/info/sites/info/files/styleguide_english_dgt_en.pdf>`_.
In general we try to have a relaxed and concise approach to the language.
.. _contribexample:
Examples
===============
The source for C++ examples are located in ``/example`` and
Python examples in ``/python/example``. You can add yours in the same
way you would contribute code, please see the :ref:`contribpr` section.
.. todo::
This page is under construction.
.. _contribindex:
Contributing to Arbor
=====================
First off, thank you for considering contributing to Arbor! It’s people
like you that make Arbor such a great tool. Feel welcome and read the
following sections in order to know how to ask questions and how to work
on something.
.. _contribindex-types:
Types of contributions
----------------------
There are many ways to contribute: writing tutorials or blog posts,
improving the documentation, helping other users, submitting bug reports
and feature requests or writing code which can be incorporated into
Arbor itself.
Use the `Arbor Discussions <https://github.com/arbor-sim/arbor/discussions>`__
page for support questions.
.. _contribindex-github:
Github
------
Arbor uses Git for version control and Github to host its code
repository and nearly all of its infrastructure.
- If you’re not familiar with Git or Github, please have at look at
`this introduction <https://docs.github.com/en/free-pro-team@latest/github/getting-started-with-github/set-up-git>`__.
- Make sure you have a `GitHub
account <https://github.com/signup/free>`__.
.. _contribindex-discuss:
Start a discussion
~~~~~~~~~~~~~~~~~~
You can use the `Arbor
Discussions <https://github.com/arbor-sim/arbor/discussions>`__ to help
other users of Arbor, or get help with you own simulations. Also, we’re
eager to discover what you’re using Arbor for, so don’t hesitate to
share your Arbor simulations or publications!
.. _contribindex-fileissue:
Filing an issue
~~~~~~~~~~~~~~~
If you have found a bug or problem in Arbor, or want to request a
feature, you can use our `issue
tracker <https://github.com/arbor-sim/arbor/issues>`__. If you issue is
not yet filed in the issue tracker, please do so and describe the
problem, bug or feature as best you can. You can add supporting data,
code or documents to help make your point.
.. _contribindex-solveissue:
Help solving an Issue
~~~~~~~~~~~~~~~~~~~~~
If you want to help solve an issue, that’s great! Issue labels can help
you find an something that fits your expertise or interest, and if
you’re just getting started, you can look for the “`good 1st
issue <https://github.com/arbor-sim/arbor/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+1st+issue%22>`__”
label. Don’t hesitate to post a comment to the Issue if you want to ask
clarifying questions. If you have implemented a fix, you can make a pull
request.
Write code
~~~~~~~~~~
See :ref:`contribpr`.
Add examples or unit tests
~~~~~~~~~~~~~~~~~~~~~~~~~~
See :ref:`contribexample` and :ref:`contribtest`.
Modify documentation
~~~~~~~~~~~~~~~~~~~~
See :ref:`contribdoc`.
Making a pull request
~~~~~~~~~~~~~~~~~~~~~
See :ref:`contribpr`.
.. _contribindex-other:
Other contributions
-------------------
We're always happy to hear about Arbor being used! Have you used Arbor to create a model?
Have you used Arbor in a publication? Do you have a workflow or tool that involves Arbor?
Get in touch and we'd be very happy to showcase your work through our channels.
.. _contribindex-contact:
Get in touch
------------
You can reach out in the following ways:
- `Discussions <https://github.com/arbor-sim/arbor/discussions>`__. Any
questions or remarks regarding using Arbor for your research are
welcome.
- `Slack <https://mcnest.slack.com>`__. If you’re interested in
developing Arbor itself, you can visit our Slack.
- `Email <mailto:arbor-sim@fz-juelich.de>`__.
.. _contribpr:
PR workflow
===========
.. _contribpr-make:
Making a pull request
---------------------
If you want to contribute code that implements a solution or feature,
the workflow is as follows:
1. Fork the Arbor repository
2. Create a branch off of master and implement your feature
3. Make a pull request (PR) and refer to the issue(s) that the PR
addresses.
4. An administrative matter: Arbor requests `an explicit copyright
assignment <https://en.wikipedia.org/wiki/Copyright_transfer_agreement>`__
on your contribution. This grants us the right to defend copyright on
your contributions on your behalf, and allows us to change the
license on your contribution to another open source license should
that become necessary in the future. You can upload a filled out copy
of the agreement as a file attached to the PR, or if you prefer not
to disclose private information, you can send it to
`arbor-sim@fz-juelich.de <mailto:arbor-sim@fz-juelich.de>`__.
- `Please find the Copyright Transfer Agreement
here <https://github.com/arbor-sim/arbor-materials/tree/master/copyright-transfer-agreement>`__.
5. A PR on Github is automatically tested by our CI bot called Travis.
You can also run these tests yourself by building them first
(``make tests``) and then running them (``./bin/*unit*``).
6. A member of the Arbor development team will review your contribution.
If they approve, your PR will be merged! Normally this should happen
within a few days.
Refer to the `Github
documentation <https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request>`__
for more explanation of the Git workflow described above.
.. _contribpr-review:
Reviewing a PR
--------------
Each pull request is reviewed according to these guidelines:
- At least one core Arbor team member needs to mark your PR with a
positive review. Some tips to help you get a positive review:
- You can use labels to categorize your PR. For very large PRs, it
is likely the reviewer will have many questions and comments. It
can be helpful to reach out on our Slack, Discussions or by email
beforehand if you’ve big things planned for Arbor!
- Commit logical units with clear commit messages.
- Add change summary to the PR message. If you remembered to commit
logical units, this could simply be a bulleted list of all commit
messages in the PR. If during the review process you make changes
to the features in the PR, keep the PR description updated.
- Make sure your code conforms to our `coding
guidelines <https://github.com/arbor-sim/arbor/wiki/Coding-Style-Guidelines>`__
- If you add functionality, please update the documentation
accordingly.
- If you add functionality, add tests if applicable. This helps make
sure Arbor is stable and functionality does what it’s supposed to
do.
- If you make changes to GPU simulations, you can post a reply to
your PR with ``bors try``. This will run our GPU test-suite.
- If you add/change the public C++ API, provide Python wrappings.
- Make sure Arbor compiles and has no new warnings.
- Travis CI must produce a favourable result on your PR.
- An Arbor team member will (squash) merge the PR with the PR change
summery as commit message.
.. _contribpr-merge:
Merging a PR
------------
- Use PR comment as commit message and verify it covers the changes in
the PR.
- ONLY squash-and-merge (Github should not allow anything else
anymore).
- The creator of a pull request should not review or merge their own
pull request.
- A reviewer can merge if their own review is favourable and other
criteria are met.
- A reviewer can poke another Arbor core team member to do the merge.
.. _contribtest:
Tests
============
.. todo::
This page is under construction.
...@@ -9,7 +9,7 @@ Welcome to the documentation for Arbor, the multi-compartment neural network sim ...@@ -9,7 +9,7 @@ Welcome to the documentation for Arbor, the multi-compartment neural network sim
You can find out how to :ref:`get Arbor<in_install>`; get started quickly with our :ref:`tutorials<gs_other_examples>`; or continue reading to learn more about Arbor. You can find out how to :ref:`get Arbor<in_install>`; get started quickly with our :ref:`tutorials<gs_other_examples>`; or continue reading to learn more about Arbor.
What is Arbor? What is Arbor?
------------- --------------
Arbor is a high-performance library for computational neuroscience simulations with multi-compartment, morphologically-detailed cells, Arbor is a high-performance library for computational neuroscience simulations with multi-compartment, morphologically-detailed cells,
from single cell models to very large networks. Arbor is written from the ground up with many-cpu and gpu architectures in mind, to from single cell models to very large networks. Arbor is written from the ground up with many-cpu and gpu architectures in mind, to
...@@ -27,6 +27,7 @@ Documentation organisation ...@@ -27,6 +27,7 @@ Documentation organisation
* :ref:`modelintro` describes the design and concepts used in Arbor. The breakdown of concepts is mirrored (as much as possible) in the :ref:`pyoverview` and :ref:`cppoverview`, so you can easily switch between languages and concepts. * :ref:`modelintro` describes the design and concepts used in Arbor. The breakdown of concepts is mirrored (as much as possible) in the :ref:`pyoverview` and :ref:`cppoverview`, so you can easily switch between languages and concepts.
* :ref:`hpc-overview` details Arbor-features for distribution of computation over supercomputer nodes. * :ref:`hpc-overview` details Arbor-features for distribution of computation over supercomputer nodes.
* :ref:`internals-overview` describes Arbor code that is not user-facing; convenience classes, architecture abstractions, etc. * :ref:`internals-overview` describes Arbor code that is not user-facing; convenience classes, architecture abstractions, etc.
* Contributions to Arbor are very welcome! Under :ref:`contribindex` describe conventions and procedures for all kinds of contributions.
Citing Arbor Citing Arbor
------------ ------------
...@@ -65,3 +66,14 @@ Arbor is an `eBrains project <https://ebrains.eu/service/arbor/>`_. ...@@ -65,3 +66,14 @@ Arbor is an `eBrains project <https://ebrains.eu/service/arbor/>`_.
cpp/index cpp/index
hpc/index hpc/index
internals/index internals/index
.. toctree::
:caption: Contributing:
:maxdepth: 1
contrib/index
contrib/pr
contrib/coding-style
contrib/doc
contrib/example
contrib/test
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment