v0.8.23
Coding practice

Naming conventions and coding policies and principles in MoFEM.

MoFEM and User Modules

All new finite elements or modification or finite elements are implemented in users modules (UM). In future it will be always single version of MoFEM library but many UM forks. UM are developed independently from MoFEM library repository. How to add user modules see How to add user module?

Repository Commits

As a general rule, developers should update frequently, and commit changes often. However, the repository should always remain in a state where the code can be compiled. Most of the time, the code should also successfully execute "ctest" run from the top-level directory. If you commit code that violates this principal, it should be your first priority to return the repository code to a compilable state and next to make sure that all tests (ctest) runs without errors.

Intermediate development stages should be committed by pull-request to develop branch. The committed code has to compile, and committer should verify if tests are running successfully.

Finished portion of work should be committed by pull-request to CDashTesting branch. Commits to the CDashTesting branch should also come with a non-trivial, useful, non-verbose log message. It is required that before pull request user merge current CDashTesting branch and verify if all tests run without fails. If pull request is accepted it is user responsibility to verify results on CDash server http://cdash.eng.gla.ac.uk/cdash/. The first priority will be to eliminate compilation errors, completion warnings, failed tests and memory leaks.

Some guidance about branches:

  • Make often updates (git pull)
  • Do not commits to other people branch. You can commit only to branches created by yourself.
  • If you like to commit to other (not own created) branch you have to do pull request.
  • Before marking pull request, pull from branch to which you like to commit.
  • Pull regularly form CDashTesting branch.
  • If you working on two different tasks make two different branches. This simplifies code revision.
  • We have use three major branches, i.e. major branch, CDashTesting branch and develop branch.
  • User branches should have name as follows john/branch_name, for example lukas/work_on_new_aprox_base.

Code Style and Best Practices

MoFEM code should follow MoAB code style and best practices listed here http://www.mcs.anl.gov/~fathom/moab-docs/html/styleguide.html.

  • Style:
  • Code formatting style makes code easier to read and follow by other. For that reason, we choose to use LLVM style, i.e. one of the built-in styles offered by clang-format. The motivation for use this style is as follows; is popular, can be set-up in many editors and compact.
  • Make indentations. Indent code to better convey the logical structure of your code. Without indenting, code becomes difficult to follow.
    if (...) {
    if (...) {
    ...
    } else {
    ...
    }} else if {
    ...
    } else {
    ...
    }
    Can you follow this ? This is much better
    if (...) {
    if (...) {
    ...
    } else {
    ...
    }
    } else if {
    ...
    } else {
    ...
    }
    • Enable syntax highlighting in your text editor.
    • Break large, complex sections of code into smaller, comprehensible modules (subroutine/functions/methods). A good rule is that modules do not exceed the size of the text editor window.
    • Indentation should have TWO SPACES. You have to set up your favorite editor to make TWO SPACES for TAB.
    • Use empty lines to provide organizational clues to source code, blocks (paragraphs -like structure) help the reader in comprehending the logical segmenting.
  • Types:

    Use types to indicate what it is, and the name of the variable what it is. For example

    std::vector<PetscLocalDofIdx> on_boundary;

    In above one can see that you will be going to have local indices, and the vector will store indices on indices on boundary only.

    An alternative to the example above you can see less informative, but equivalent definition below

    std::vector<int> local_petsc_indices_on_boundary;

    For MoFEM data types see MoFEM::Types.

  • Names:
    • A name should tell what rather than how, avoid names that expose underlying implementation.
    • Class names should be in the CamelBack style, e.g. EdgeMesh or VertexMesher.
    • Class member variables should be camelBack, e.g. EdgeMesh::schemeType; each member variable, e.g. int memberVariable, should have set/get functions void member_variable(int newval) and int member_variable(), respectively
    • Abstract class members should be abstract_class_member, e.g. set_field_orer()
    • Enumeration values should be all capitalized, with underscores avoided if possible (the enumeration name indicates the general purpose of the enumeration, so e.g. we use EQUAL, not EQUAL_MESH)
    • It is advised that names of vectors should start with "v", e.g. VectorDouble v_coordinate_vector; VectorDouble vCoordinateVector, respectively for local variables and class members.
    • It is advised names of matrices should start with "m", e.g. MatrixDouble m_deformation_gradient; MatrixDouble mDeformationGradient, respectively for local variables and class members.
    • Use a verb-noun method to name routines that perform some operation-on-a-given-object. Most names are constructed by concatenating several words, use mixed-case formatting or underscore to ease reading.
      calculateKineticEnergy ( . . . )
      calculate_kinetic_energy ( . . . )
      or any other derivatives.
    • Avoid elusive names, open to subjective interpretation like
      Analyze ( . . . ) / / subroutine or function or method
      nnsmcomp1 / / variable
    • Each class header should be fully commented.
    • A \file comment block at the top of the file; DO NOT include things like Author and Date blocks; this stuff is available from subversion if we really need to know.
    • Each function in both the public and private interfaces should be commented, INCLUDING ANY ARGUMENTS AND RETURN VALUES. See the MOAB classes for examples of how to format these comments.
    • Developers should avoid using #include in header files, as they propagate dependencies more widely than necessary.
    • Local variables and function argument have names with small letters, i.e. temperature_val, nodal_position.
    • If variable is a pointer, it should have name as follows
      class A {
      boost::shared_ptr<double> valPtr; ///< class member variable
      };
      void f() {
      boost::shared_ptr<double> val_ptr; ///< local variable\
      }
    • All FTensor variables should be name as follows
      class A {
      FTensor::Tensor1<double,3> tForce; ///< class member variable
      }
      void f() {
      FTensor::Tensor1<double,3> t_force; ///< local variable
      }
    • Append/Prepend computation qualifiers like Av, Sum, Min, Max and Index to the end of a variable when appropriate.
    • If you commit your code remove are depreciated functions names. Use of OLD function name (or old functions arguments) generate compilation warring, f.e.
      In file included from mofem/users_modules/basic_finite_elements/src/impl/DirichletBC.cpp:39:0:
      mofem/src/interfaces/Interface.hpp: In member function 'MoFEMErrorCode MoFEM::Interface::set_other_local_VecCreateGhost(const MoFEM::Problem*, const std::string&, const std::string&, RowColData, Vec, InsertMode, ScatterMode, int)':
      mofem/src/interfaces/Interface.hpp:1300:107: warning: 'MoFEMErrorCode MoFEM::Interface::set_other_local_VecCreateGhost(const MoFEM::Problem*, const std::string&, const std::string&, RowColData, Vec, InsertMode, ScatterMode, int)' is deprecated (declared at mofem/src/interfaces/Interface.hpp:1296) [-Wdeprecated-declarations]
      ierr = set_other_local_VecCreateGhost(problem_ptr,fiel_name,cpy_field_name,rc,V,mode,scatter_mode,verb); CHKERRQ(ierr);
    • Constants and Macros
      • Don't use a pre-processor macro where a const variable or an inline or template function will suffice. There is absolutely benefit to the former over the later with modern compilers. Further, using macros bypasses type checking that the compiler would otherwise do for you and if used in headers, introduce names into the global rather than MoFEM namespace.
      • Don't define constants that are already provided by standard libraries. For example, use M_PI as defined in math.h rather than defining your own constant.
  • Each header file should have defined macro, following example
    #ifndef __CLASS_NAME_HPP__
    #define __CLASS_NAME_HPP__
    class ClassName {
    };
    #endif //__CLASS_NAME_HPP__
  • Each function should have build in error checking, following example
  • About pointers:
    • Read it backwards (as driven by Clockwise/Spiral Rule):
      int* a; // pointer to int
      int const * b; // pointer to const int
      int * const c; // const pointer to int
      int const * const d; // const pointer to const int
    • Now the first const can be on either side of the type so:
      const int * a; // == int const *
      const int * const b; // == int const * const
    • If you want to do pointers to pointers:
      int ** a; // pointer to pointer to int
      int ** const b; // a const pointer to a pointer to an int
      int * const * c; // a pointer to a const pointer to an int
      int const ** d; // a pointer to a pointer to a const int
      int * const * const f; // a const pointer to a const pointer to an int
  • Memory allocation
    • USE VALGRIND. Valging is powerful tool to find execution errors, use it if your code behave differently on two different computers, or you get segmentation fault, ect. Valgrind can be used as follows
      valgrind --track-origins=yes ./program_name -my_file mesh.cub
      Use small mesh, i.e. small problem, when you run valgrind, it take some time. YOU NEED TO COMPILE CODE WITH -DCMAKE_BUILD_TYPE=Debug.
    • Keep array of objects in STL vectors, multi-indexes or any other Boost or STL data structures. AVOID ALLOCATING ARRAY OF OBJECTS ON HEAP USING REGULAR POINTERS.
    • Use smart pointers http://www.boost.org/doc/libs/1_57_0/libs/smart_ptr/smart_ptr.htm. If existing code uses regular pointer, take opportunity and change it to smart pointer.
    • Use MoFEM::SmartPetscObj to create PETSc objects. SmartPetscObj will mangage ownership and destrcution of PETSc objects like Vec, Mat, KSP, SNES, IS and all others.
    • Check program with line command argument -log_view, verifying if number of created PTESc objects is equal to number of destroyed objects
      Memory usage is given in bytes:
      Object Type Creations Destructions Memory Descendants' Mem.
      Reports information only for process 0.
      --- Event Stage 0: Main Stage
      Matrix 8 8 1526844 0
      Matrix Partitioning 1 1 660 0
      Index Set 40 40 42992 0
      IS L to G Mapping 11 11 32016 0
      Vector 68 68 361384 0
      Vector Scatter 16 16 9792 0
      SNES 1 1 1340 0
      SNESLineSearch 1 1 880 0
      DMSNES 1 1 680 0
      Krylov Solver 1 1 18960 0
      DMKSP interface 1 1 664 0
      Preconditioner 2 2 2104 0
      Distributed Mesh 2 2 9008 0
      Star Forest Bipartite Graph 5 5 4136 0
      Discrete System 2 2 1632 0
      Viewer 1 0 0 0
  • Make member functions of class constant always when possible. Try to code that you can create such class members. Constant member function does not a changea state of the class. So it does not influence results or actions of another member of that class. That simplifies searching for the bug, testing and overall limits likelihood of the error. Example of constant class member
    struct {
    MoFEMErrorCode someFunction(const int some_argument,const double &some_return_value) const {
    }
    };
  • Use static_cast<> and avoid C style casting. The compiler checks C++ style casts. C-style casts aren't and can fail at runtime also, C++ style casts can be searched for easily, whereas it's hard to find C style casts. Another big benefit is that the four different C++ style casts express the intent of the programmer more clearly. When writing C++, always use the C++ ones over the C style. If you see the C-style cast, please take the opportunity to fix this.
  • Use nullptr instead NULL as a default argument in the function. nullptr keyword designates an rvalue constant that serves as a universal null pointer literal, replacing the buggy and weakly-typed literal 0 and the infamous NULL macro.
  • C++ Partial Template Specialization, see https://stackoverflow.com/a/6142796/8043075 for details.
    template<typename A, typename B>
    struct TwoTypes { };
    template<typename A, typename B>
    struct X {
    /* forwards ... */
    void f() { fImpl(TwoTypes<A, B>()); }
    /* special overload for <A, int> */
    template<typename A1>
    void fImpl(TwoTypes<A1, int>) {
    /* ... */
    }
    /* generic */
    template<typename A1, typename B1>
    void fImpl(TwoTypes<A1, B1>) {
    /* ... */
    }
    };
    Explicitly specializing functions is never (almost never?) the right way. Never explicitly specialized a function template. Overloading and partial ordering is superior.