Best Practices

Table of Contents

• Use issues to discuss intended modifications
• Document your code
• Coding style
• MPI wrappers
• Please do NOT use MPI BARRIERS unless you know exactly what you are doing
• External tools and libraries
• Unit testing and regression testing
• Wiki usage

Use issues to discuss intended modifications

GitHub provides a system to track issues. It should be a central place to monitor SPECFEM3D evolution. In particular:

• report bug as they occur

• plan modifications.

The issue tracker interface lets us track bugs being fixed and enhancements being made by assigning labels. We will reserve the labels

• bug for issue fixing

• enhancement for feature development.

Document your code

Any new code should be fully Doxygen commented. If you have some free time, feel free to comment any code you modify.

Coding style

When modifying an existing file, try to maintain consistency with its original style. If the code you add looks drastically different from the original code, it may be difficult for readers to follow. Try to avoid this.

As a general guideline, we recommend the following code formatting style (examples are given in fortran90 since it is mostly used in the package, but this would hold for other languages as well):

give space for breathing:

good

  dx = 0.5 * fac * (a - b)

bad

  dx=1/2*fac*(a-b)

Note that in performance critical sections, please use multiplication by 0.5 rather than divide by 2 for floating-points.

use consistent 2-space indents:

good

  if (i == 1) then
    print *,'great'
  endif

bad

  if(i == 1)then
        print *,'not so great'
  endif

start your code with an indent:

good

  subroutine scotch_partitioning()
  implicit none
  ..

bad

subroutine scotch_partitioning
  implicit none
  ..

The line beginning should only be used for very important sections, as it makes the line very prominent to read. For example, only use it for function descriptions, important comments, or file headers. For comments, see also next point...

our exception, module definitions start at beginning:

good

module my_par
  integer :: count
end module

bad

  module my_par
  integer :: count
  end module  

comment, comment, comment your code:

good

  ! gets associated normal on GLL point
  ! (note convention: pointing outwards of acoustic element)
  nx = coupling_ac_el_normal(1,igll,iface)

  ! continuity of displacement and pressure on global point
  accel(1,iglob) = accel(1,iglob) + jacobianw*nx*pressure  

bad

  nx = coupling_ac_el_normal(1,igll,iface)
  accel(1,iglob) = accel(1,iglob) + jacobianw*nx*pressure

Note we prefer indenting the comments as well to make it easier for reading the code, e.g., when inside multiple if-then statements. Putting the comment at the beginning breaks the flow...

(the only exception to this rule is made by Dimitri)

comment, comment, comment your functions:

good

  subroutine get_cg_direction_tiso()

! calculates TI gradient based on a conjugate gradient method
!
! based on: Tarantola, inverse problem theory, 2005.
!                  section 6.22.7 conjugate directions, page 217.
!                  formula for alpha_n based on Polak & Ribiere (1969)
!
! note: we use a preconditioner F_0 = 1, thus lambda_n = gamma_n in (6.322)
!          and use gamma_n as the smoothed kernel (for bulk_c, bulk_betav,..).
!
!          however, one could see smoothing as preconditioner F_0, thus
!          gamma_n would be un-smoothed kernel and lambda_n would be smoothed one...
!          i'm not sure if this makes a difference.

  ..

bad

  subroutine get_cg_direction_tiso()

! CG step

  ..

Note that we haven't been very strict in adopting a doxygen-readable function declaration. It is nice though to have it. For example, you would do:

!> Define an ADIOS scalar integer variable and autoincrement the adios
!! group size by (4).
!! \param adios_group The adios group where the variables belongs
!! \param group_size_inc The inout adios group size to increment
!!                       with the size of the variable
!! \param path The logical path structuring the data and containing
!!             the variable
!! \param name The variable name in the ADIOS file.
!! \param var The variable to be defined. Used for type inference. Can be
!             ignored.
!!
!! \note See define_adios_double_scalar()
  subroutine define_adios_integer_scalar(adios_group, group_size_inc, path, name, var)
  ..

use double-colons for parameter declarations:

good

  integer :: i,j,k

bad

  integer i,j,k

use separators between subroutines:

good

  ..
  end subroutine

!
!----------------------------------------------------------
!

  subroutine get_color(icolor)
  ..

bad

  ..
end subroutine

subroutine get_color(icolor)
  ..

MPI wrappers

When programming new features or routines for SPECFEM3D or SPECFEM3D_GLOBE, if you need to use MPI, please do not call MPI directly but rather use the wrapper routines that you will find in file src/shared/parallel.f90.

Please do NOT use MPI BARRIERS unless you know exactly what you are doing

In the past we sometimes got deadlocking in the code because of MPI_BARRIERS that were added recently inside "if" statements. This is pretty dangerous because if the "if" statement is true on some processors but not others the code deadlocks and the simulation becomes idle (forever).

Thus, please do not add MPI_BARRIERs unless you know exactly what you are doing (in 99% of the cases they are useless anyway).

In particular, never put them in "if" statements.

External tools and libraries

Important note for all developers: we should never use Release Candidate (rc) versions of any external package used by the SPECFEM codes, only stable versions. For instance, regarding SCOTCH, please do not use e.g. scotch_6.0.1rc2 but rather a stable version such as scotch_5.1.12b.

Unit testing and regression testing

This section is just a stub.

Wiki usage

The Wiki is meant to be used as a developer reference, not a replacement for the user manual. In general, the Wiki should only contain entries on software engineering and computer science aspects of SPECFEM development.

Anyone with pull access to the central repository is allowed to make changes to the Wiki. Note that the Wiki is itself a Git repository that you can clone locally, modify with your favourite Markdown editor and push your changes to the remote server.