contribution-guide.rst 8.16 KB
Newer Older
1
.. contribution-guide:
2

3
##################
4
Contribution guide
5
##################
6

7 8 9 10 11 12 13 14
License
=======

All contributed code must be compatible with the `Apache 2`_ license,
preferably by being contributed under the Apache 2 license. Code
contributed with another license will need the license reviewed by
Intel before it can be accepted.

15

16
Code formatting
L.S. Cook's avatar
L.S. Cook committed
17
===============
18

19 20 21
All C/C++ source code in the repository, including the test code, must
adhere to the source-code formatting and style guidelines described
here.  The coding style described here applies to the nGraph
L.S. Cook's avatar
L.S. Cook committed
22
repository. Related repositories may make adjustments to better match
23 24
the coding styles of libraries they are using.

25

26 27
Adding ops to nGraph Core
-------------------------
28

29 30 31 32
Our design philosophy is that the graph is not a script for running
optimized kernels; rather, the graph is a specification for a
computation composed of basic building blocks which we call
``ops``. Compilation should match groups of ``ops`` to appropriate
L.S. Cook's avatar
L.S. Cook committed
33
optimal semantically equivalent groups of kernels for the backend(s)
34 35
in use. Thus, we expect that adding of new Core ops should be
infrequent and that most functionality instead gets added with new
36
functions that build sub-graphs from existing core ops. 
37 38


39
Coding style
40 41
-------------

42 43 44 45 46
We have a coding standard to help us to get development done. If part of the
standard is impeding progress, we either adjust that part or remove it. To this
end, we employ coding standards that facilitate understanding of *what nGraph
components are doing*. Programs are easiest to understand when they can be
understood locally; if most local changes have local impact, you do not need to
47 48
dig through multiple files to understand what something does and if it
is safe to modify.
49 50 51 52

Names
~~~~~

53 54
Names should *briefly* describe the thing being named and follow these casing
standards:
55 56 57

- Define C++ class or type names with ``CamelCase``.
- Assign template parameters with ``UPPER_SNAKE_CASE``.
58
- Case variable and function names with ``lower_snake_case``.
59

60
Method names for basic accessors are prefixed by ``get_``, ``is_``, or ``set_`` and
61
should have simple :math:`\mathcal{O}(1)` implementations:
62

63
- A ``get_`` method should be externally idempotent. It may perform some simple
64 65 66
  initialization and cache the result for later use.  Trivial ``get_``
  methods can be defined in a header file. If a method is
  non-trivial, that is often a sign that it is not a basic accessor.
67

68
- An ``is_`` may be used instead of ``get_`` for boolean accessors.
69

70
- A ``set_`` method should change the value returned by the corresponding ``get_``
71
  method.
72

73 74 75 76
  * Use ``set_is_`` if using ``is_`` to get a value.
  * Trivial ``set_`` methods may be defined in a header file.

- Names of variables should indicate the use of the variable.
77

78 79 80 81 82
  * Member variables should be prefixed with ``m_``.
  * Static member variables should be rare and be prefixed with ``s_``.

- Do not use ``using`` to define a type alias at top-level in header file.
  If the abstraction is useful, give it a class.
83

84 85 86 87 88 89 90 91 92 93
  * C++ does not enforce the abstraction. For example if ``X`` and ``Y`` are
    aliases for the same type, you can pass an ``X`` to something expecting a ``Y``.
  * If one of the aliases were later changed, or turned into a real type, many
    callers could require changes.


Namespaces
~~~~~~~~~~

- ``ngraph`` is for the public API, although this is not currently enforced.
94

95
  * Use a nested namespace for implementation classes.
96 97
  * Use an unnamed namespace or ``static`` for file-local names. This helps
    prevent unintended name collisions during linking and when using shared
98 99
    and dynamically-loaded libraries.
  * Never use ``using`` at top-level in a header file.
100

101 102
    - Doing so leaks the alias into users of the header, including headers that
      follow.
103
    - It is okay to use ``using`` with local scope, such as inside a class
104
      definiton.
105 106 107
  * Be careful of C++'s implicit namespace inclusions. For example, if a
    parameter's type is from another namespace, that namespace can be visible
    in the body.
108 109 110 111 112 113 114
  * Only use ``using std`` and/or ``using ngraph`` in ``.cpp`` files. ``using`` a
    nested namespace has can result in unexpected behavior.


File Names
~~~~~~~~~~

115
- Do not use the same file name in multiple directories. At least one
116 117 118 119 120 121 122
  IDE/debugger ignores the directory name when setting breakpoints.

- Use ``.hpp`` for headers and ``.cpp`` for implementation.

- Reflect the namespace nesting in the directory hierarchy.

- Unit test files are in the ``tests`` directory.
123

124
  * Transformer-dependent tests are tests running on the default transformer or
125 126 127 128
    specifying a transformer. For these, use the form

    .. code-block:: cpp

L.S. Cook's avatar
L.S. Cook committed
129
       TEST(file_name, test_name)
130 131

  * Transformer-independent tests:
132

133
    - File name is ``file_name.in.cpp``
134 135
    - Add ``#include "test_control.hpp"`` to the file's includes
    - Add the line ``static std::string s_manifest = "${MANIFEST}";`` to the top of the file.
136 137 138 139
    - Use

      .. code-block:: sh

140
         NGRAPH_TEST(${BACKEND_NAME}, test_name)
141

142 143
      for each test. Files are
      generated for each transformer and the ``${BACKEND_NAME}`` is replaced
144 145
      with the transformer name.

146 147 148 149
      Individual unit tests may be disabled by adding the name of the test to the
      ``unit_test.manifest`` file found in
      the transformer's source file directory.

150 151 152 153

Formatting
~~~~~~~~~~

154 155
Things that look different should look different because they are different. We
use **clang format** to enforce certain formatting. Although not always ideal,
156 157
it is automatically enforced and reduces merge conflicts.

158
- The :file:`.clang-format` file located in the root of the project specifies
159 160 161 162 163 164
  our format.  Simply run:  

  .. code-block:: console

     $ make style-check
     $ make style-apply
165 166

- Formatting with ``#include`` files:
167

168 169 170
  * Put headers in groups separated by a blank line. Logically order the groups
    downward from system-level to 3rd-party to ``ngraph``.
  * Formatting will keep the files in each group in alphabetic order.
171
  * Use this syntax for files that **do not change during nGraph development**; they
172
    will not be checked for changes during builds. Normally this will be
173 174 175 176 177
    everything but the ngraph files:

    .. code-block:: cpp

       #include <file>
178

179
  * Use this syntax for files that **are changing during nGraph development**; they will
L.S. Cook's avatar
L.S. Cook committed
180
    be checked for changes during builds. Normally this will be ngraph headers:
181 182 183 184 185 186 187 188 189 190 191

    .. code-block:: cpp

       #include "file"

  * Use this syntax for system C headers with C++ wrappers:

    .. code-block:: cpp

       #include <c...>

192
- To guard against multiple inclusion, use:
193 194 195 196 197

  .. code-block:: cpp

     #pragma once

198 199 200
  * The syntax is a compiler extension that has been adopted by all
    supported compilers.

201 202 203 204 205 206
- The initialization

  .. code-block:: cpp

     Foo x{4, 5};

L.S. Cook's avatar
L.S. Cook committed
207
  is preferred over
208 209 210 211 212

  .. code-block:: cpp

     Foo x(4, 5);

213
- Indentation should be accompanied by braces; this includes single-line bodies
214 215 216
  for conditionals and loops.

- Exception checking:
217

218
  * Throw an exception to report a problem.
219
  * Nothing that calls ``abort``, ``exit`` or ``terminate`` should be used. Remember
220 221
    that ngraph is a guest of the framework.
  * Do not use exclamation points in messages!
222 223
  * Be as specific as practical. Keep in mind that the person who sees the error
    is likely to be on the other side of the framework and the message might be
224 225
    the only information they see about the problem.

226 227
- If you use ``auto``, know what you are doing. ``auto`` uses the same
  type-stripping rules as template parameters. If something returns a reference,
228
  ``auto`` will strip the reference unless you use ``auto&``:
229

230 231 232 233 234 235
  * Don't do things like

    .. code-block:: cpp

       auto s = Shape{2,3};

L.S. Cook's avatar
L.S. Cook committed
236
    Instead, use
237

L.S. Cook's avatar
L.S. Cook committed
238
    .. code-block:: cpp
239

L.S. Cook's avatar
L.S. Cook committed
240
       Shape s{2, 3};
241

242
  * Indicate the type in the variable name.
243

244
- One variable declaration/definition per line
245

246
  - Don't use the C-style
247

248
    .. code-block:: cpp
249

250
       int x, y, *z;
251

252
    Instead, use:
253

254
    .. code-block:: cpp
255

256 257 258
       int x;
       int y;
       int* z;
259

260

261 262 263
To contribute documentation for your code, please see the :doc:`doc-contributor-README`. 


264
.. _Apache 2: https://www.apache.org/licenses/LICENSE-2.0
L.S. Cook's avatar
L.S. Cook committed
265
.. _repo wiki: https://github.com/NervanaSystems/ngraph/wiki