Coding standards
In all contributions to the code, please follow the spirit of these guidelines as much as possible, and try to rectify violations you come across. If you think there is a good reason to have an exception to these rules in some specific part of the code, please discuss it with the other developers first.
Guidelines for Committing
- Do not commit a line longer than 132 characters, which may cause trouble for some compilers. This constraint is enforced by the buildbot. Also no line after pre-processing may exceed that length.
SAFE_ALLOCATE
is the typical problem in this regard. Use shorter variables for the dimensions if the line is too long. - Always check over your changes with git status and git diff to be sure that you really mean to change what is there. Avoid accidentally committing changes. Remember that you can commit (or diff) files or directories individually, in case you have changes in your working directory which you do not wish to commit.
- Be descriptive and verbose in your commit messages. Ideally, for all but the most trivial changes, there should be a paragraph explaining the goal of the changes and how they accomplish it. Follow the style of other commits. This allows others to follow along with what you are up to and help find any problems, and indeed will be helpful for you looking back at your changes in a few months. Make comments covering all of the changes in your commit. Use formatting that will help your message be read in GitLab.
- Correct spelling is required!
- Make many small independent commits rather than a smaller number of large ones, which makes it easier for others to see what you are doing and what the origin of any problem that arises might be (especially with the buildbot and occasionally manually searching for the commit where a certain problem arose). If you find yourself making extensive changes about different issues, stop and make changes about the different issues in separate git branches.
- Retain the history of files as much as possible in the git repository: to rename a file, use git mv rather than git add and git rm .
- Copy or repeat code as little as possible as it makes the code harder to understand and maintain. You can generally reuse the same code with if conditions or pulling out the part you want to use again as a subroutine.
- Don’t change indentation at the same time that you make meaningful changes, or else it difficult to see what meaningful changes were actually made. Use two separate commits for clarity.
- If during a git merge or git rebase you find your local version to be in conflict, be very careful about resolving it to be sure that you are not undoing other recent work in the repository. If you have trouble or are unsure how to proceed, ask for help.
- If you introduce verbatim code from some external source, document carefully in the code itself and in your commit message where it came from and what its licensing provisions are (and how they are compatible with our GPL v2 license – https://www.gnu.org/philosophy/license-list.html-GPLCompatibleLicenses). If you import a whole directory, put it in external_libs , and document the origin and license in a README.octopus .
- If you make a significant change to a file, make sure the copyright notice at the beginning of the file is up to date (i.e., it includes your name and the name of any other person that contributed to the changes you are commiting).
- Files that are generated by compilation and installation should be listed in the .gitignore file so that they do not show up with git status .
- When you commit a bugfix, clearly identify it by beginning the commit message with “Bugfix:”. State when the bug was introduced (if you are able to determine that) and what situations it would affect, and how your change rectifies the situation. Whenever possible, make sure that there is a test or test match that ensures the correct behavior.
- Branches to apply bugfixes should be created from the main branch. See Workflow for a detailed explanation.
- Remember to add new files to the relevant Makefile.am , including testsuite .test and .inp files.
- Pay attention to compiler warnings, at compile time and run time, on your own machine and from the buildbot workers, and try to fix them when possible: they often reveal a latent problem, even if tests are not failing.
- When you implement a method or algorithm, be sure to provide a citation in a comment (to papers, websites, etc.) and in the variable documentation when possible.
- You don’t necessary have to cover every possible situation when you write some new code, but you must note and enforce whatever assumptions you make, so it is clear to others (and yourself next year!) what cases are not implemented. Also, add an assertion or a fatal message to prevent the execution of parts of the code that do not work in a specific case.
Special Octopus infrastructure
When you add new input variables, be sure to write a variable description.
- You can use HTML formatting such as <i>, <tt>, <a href=...>.
- Enclose LaTeX math in $ .. $ tags, which will be parsed by MathJax (http://docs.mathjax.org/en/latest/tex.html).
- Specify a Section (and subsection(s) if appropriate) to organize the variable list.
- Types: float, integer, flag (means binary bits summed), string, logical, block.
- Listing Option x automatically creates an integer constant
OPTION_X
usable in Fortran with the corresponding value. - For a flag type, the option values can be specified as bit(0), bit(1), etc. to generate the series of powers of 2.
- List the default in the Default field if it can be stated simply; otherwise, describe it in the Description block.
- Synonyms: an option without a corresponding description will not appear in the variable reference, but will still be understood by the parser. This can be used to leave an obsolete option for backward compatibility.
General form:
!%Variable NAME
!%Type TYPE
!%Default VALUE
!%Section SECTION[::SUBSECTION]
!%Description
!% DESCRIPTION TEXT
!%Option OPTION_NAME [OPTION_VALUE]
!% OPTION_DESCRIPTION
!%Option OPTION_NAME2 [OPTION_VALUE2]
!% OPTION_DESCRIPTION2
!%End
Example, with following call to parser:
!%Variable ExcessCharge
!%Type float
!%Default 0.0
!%Section States
!%Description
!% The net charge of the system. A negative value means that we are adding
!% electrons, while a positive value means we are taking electrons
!% from the system.
!%End
call parse_variable('ExcessCharge', M_ZERO, excess_charge)
- References should be in APS style, including formatting tags: Author1, Author2, et al., J. Short Title Vol, page (year).
- Parts of the code that are new and not fully tested, or old and known to have problems, should be marked with calls to messages_experimental, and there should also be a corresponding comment in the variable documentation to identify what is experimental for the users. Users will be pointed to the page [[Experimental Features]] by resulting warnings.
- If you remove a feature or any significant block of meaningful code, list that fact here on the wiki: [[Developers:Removed Features]]
- If you make a big change regarding backwards compatibility, list that here on the wiki: [[Developers:Big changes]]
- Character variables representing a filename should be declared with length
MAX_PATH_LEN
. - If you rename, remove, or replace a variable, add a call to messages_obsolete_variable to help users with migration.
- Use constant
MAX_DIM
in static allocation rather than assuming a maximum dimensionality. HoweverSAFE_ALLOCATE
should always use to the actual dimension. - All routines, functions, interfaces, constants should be in a module.
‘‘Unless otherwise specified, the rules below apply to Fortran source. Some can be applied to C source as well.’’
Required Elements
-
Every subroutine, function, module, and program must begin with
implicit none
andprivate
(for default scope). Module routines should not use any modules or containimplicit none
, which should be at module scope instead. Modules containing only public parameters, types, and/or interfaces may ave defaultpublic
scope. Modules should list what ought to be publicly accessible viapublic ::
, but should not list anything explicitly sprivate
since this should be the default. All interfaces should containimplicit none
. -
All source files must have the line
-include
, and include the moduleglobal_m
. -
Every subroutine and function must have
PUSH_SUB(routine_name)
as the first executable line (‘‘i.e.’’ th earliest position that will compile, after all the variable declarations) and havePOP_SUB(routine_name)
as the last line before anyreturn
statements. A subroutine contained within another should use this style:PUSH_SUB(routine_name.contained_name)
. If there are multiple possible exit points from a routine, each must have aOP_SUB
. Exceptions:- the push_sub and pop_sub routines themselves and any routines they use. Add the comment
! this is called by push/pop so there cannot be a push/pop in this routine
- any routine called before the debug level has been set;
- certainroutines that are called so many times as to swamp the debugging output. Add the comment
! no push/pop since called too frequently
elemental
orpure
routines, where addition ofPUSH_SUB
/POP_SUB
will not compile;- routines that are supposed to be threadsafe – add the comment
! no push_sub, threadsafe
- the push_sub and pop_sub routines themselves and any routines they use. Add the comment
-
Do not use
return
for subroutines that just end at the end of the code. -
Each file should begin with a license header of this form:
!! Copyright (C) 2004-2021 <Authors> !! !! This program is free software; you can redistribute it and/or modify !! it under the terms of the GNU General Public License as published by !! the Free Software Foundation; either version 2, or (at your option) !! any later version. !! !! This program is distributed in the hope that it will be useful, !! but WITHOUT ANY WARRANTY; without even the implied warranty of !! MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the !! GNU General Public License for more details. !! !! You should have received a copy of the GNU General Public License !! along with this program; if not, write to the Free Software !! Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA !! 02110-1301, USA.
-
Every .F90 file should end with these comments that set the emacs mode:
!! Local Variables: !! mode: f90 !! coding: utf-8 !! End:
Program Structure
- Octopus is written using the Fortran 95 standard. Fortran 2003/2008 features are allowed, provided that they are supported by all the compilers being used in buildbot the test farm.
- We use exactly two spaces for indentation, for the body of blocks such as
if
anddo
, and for continuation lines. Do not use tabs anywhere in source code. The largest block in the file (typicallymodule
,program
, orsubroutine
) should begin in column zero. Preprocessor directives (‘‘e.g.’’-ifdef
,-include
) should be in column zero. The keywordcontains
should be at the same indentation as the beginning and end of the program unit. (See “Editor Configuration” below for help with this.) - All named program blocks (‘‘e.g.’’
program
,subroutine
,function
,module
,interface
) should repeat the name in theend
statement. ‘‘e.g.’’module basic_oct_m ... end module basic_oct_m
. - Intrinsics and keywords should be written in lower case. Preprocessor macros and parameters should be written in all upper case.
- Write
end do
andend if
notenddo
andendif
. - Single
if
clauses are preferred, but only if they fit in a single line. If more than one line is required, use theif
,then
,end if
form. Using a continuation mark&
between the condition and the statement is not allowed as this is misleading and can lead to errors when somebody assumes that the indentation keeps a statement inside the if, as it is done in some scripting languages.
Names
- Variable names should not be a single letter, since this makes it very hard to search for where it is used or modified. See https://github.com/jornada/DOENDO if you need a quick tool to rename single-letter variables. Exceptions are possible, if the name are clear (e.g. physical variables), or of limited scope (e.g. counters of small loops)
- Names of variables, arguments, functions, routines, modules, and programs should be descriptive and comprehensible, and if appropriate, comments should explain what they are.
- Do not use the same name for different entities.
- Files that are included must end with _inc .
Data Structures
- Types are named with the suffix
_t
. - If it is possible and doesn’t degrade performance, declare types as private and define access functions.
- Make
_init
and_end
routines to allocate and initialize, and deallocate and clean up types. - Do not write default initialization in the declaration of variables part of a module or type.
Declarations
- All subroutines and functions must list the argument types in the same order as in the call, using one line per argument, and declare an
intent
for each (in
whenever possible). A blank line should separate the arguments from any subsequent declarations of local variables. - Variable and argument declarations should use the separator
::
, as ininteger :: ii
; they should all be aligned. - Subroutines that follow a
contains
statement in a module or program should not specify the upper bound of array arguments, ‘‘e.g.’’ writegvec(:,:) !< (3, ng)
in preference togvec(3, ng)
. This avoids possible creation of array temporaries (wasting time and memory copying), and allows the array passed to the routine to have a larger leading dimension. However, if the lower bound is not 1, it must be specified explicitly. - Calls that pass optional arguments should call them by keyword, ‘‘i.e.’’
only_root_writes <nowiki>=</nowiki> .true.
which helps identify them as optional. A subroutine that has optional arguments must check for their presence before using them. Use of the functionsoptional_default
is encouraged. - Use Doxygen tags so that variable names and subroutine descriptions are automatically documented. See Doxygen documentation.
- Use heap not stack for variables of non-fixed size, i.e. dynamic allocation with
SAFE_ALLOCATE
. - Declare variables in double precision generally. Use single precision only in special circumstances.
- Variables must be initialized to zero after declaration or allocation if required; do not rely on the compiler for this.
- Use
parameter
variables in Fortran (orconst
in C) rather than preprocessor symbols wherever possible. - Do not declare variables as pointer unless it is necessary.
Modules
- All subroutines must be in a module.
- An F90 file should have the same name as the module it contains.
- Every module must be named with the suffix
_oct_m
, ‘‘e.g.’’global_oct_m
, and be in a file without that suffix, ‘‘e.g.’’ global.F90 . This naming scheme helps to keep the namespaces for modules, types, routines, etc. clearly separated. - Use of
only
list for module usage is highly discouraged. Use of the functionality to change the names in anonly
list is prohibited, except for when handling a module from an external library in which case it is merely highly discouraged: change the name in our code instead. - Module
use
statements should be in alphabetical order. - Routines from external libraries should have interfaces defined in a module (ideally one provided by the library itself). This module must be used in any routines making the library calls.
- MPI calls from Fortran should use the global integer
mpierr
from modulempi_m
for the (basically useless) last argument, rather than declaring its own local integer. - All modules must have
private
as default scope. The only allowed exception are modules that provide an interface to an external library, in this case the module cannot use other modules that are already used somewhere else in Octopus. - Modules can only declare as
public
objects that are defined or declared in that module.
Data types
- Use the constants defined in src/basic/global.F90
such as
M_ONE
andM_zI
, or the macroCNST
(‘‘e.g.’’CNST(0.1)
), to make sure that constants are of the correct type. - Use the variable types and other macros defined in src/include/real.F90
and complex.F90
for templated routines, and in preference to adding
-ifdef
if possible. - Handle units of measurement through the
unit_m
module, which provides routines for conversion and output, and can be used with theparse_variable
routine too for conversion of input quantities.
Output
- Correct spelling is required – one would hope this went without saying!
- Warnings should be handled with
call messages_warning
. - Errors should be handled with
call messages_fatal
. Do not usestop
orexit
. - Use
write
rather thanprint
, and supply a format rather than using*
. Inwrite
statements, do not break up string literals with continuation lines, so that the text can be grep ’d for in the code.
Comments
- Comments should be written in English.
- Correct spelling is required!
- Be sure to write explicit comments explaining the purpose and function of any unusual constructs introduced to the code, especially if they could look wrong or useless to others.
- If you comment out a piece of code, put a clear explanation about why it is commented out.
- To avoid preprocessor warnings (and XLF compilation failures), an odd number of single (‘‘i.e.’’ apostrophes) or double quotes may not be used in comments (use
`
instead). - Do not write
???
: it generates a warning from cpp about “trigraphs”. - Do not mark blank lines with
!
. - We use doxygen to document the code. See Doxygen documentation for more information.
Required Macros and Routines
- The intrinsics
allocate
anddeallocate
should never be used. Instead use the macrosSAFE_ALLOCATE
(including an explicit lower dimension so that our profiling is able to estimate the memory usage correctly) andSAFE_DEALLOCATE_A
(for arrays) andSAFE_DEALLOCATE_P
(for pointers). Danger: DO NOT use these macros in a one-lineif
statement; they must be used withif(...) then
or else some of the generated lines are outside the condition. - Use routine
io_open
instead of the intrinsicopen
when opening file units, andio_close
instead ofclose
, which will check that the operation occurs properly and write an error if not.
Forbidden
- Use of the
goto
andcontinue
keywords, as well as the relatederr
andend
parameters ofclose
,open
, andread
intrinsic functions, is forbidden. Use thecycle
andexit
keywords and theiostat
parameter instead. Keywordssequence
andcommon
are also forbidden. - Do not declare anything
external
. Use modules for functions and subroutines defined in this package, and interfaces for intrinsics and libraries. - Do not assume any value for a loop counter outside its loop. This works in C, but not necessarily in Fortran. You must save the value in another variable.
- Continuation lines shall not begin with
&
. - Use of
forall
constructs, as they have been deprecated in more recent Fortran standards. Use do loops instead.
Portability
- All code must work correctly at -O3 with gcc, PGI, and Intel compilers. If it does not, it likely contains an error even if it seems to work without optimization. Making sure it works at -O3 with other compilers too is strongly encouraged.
- Do not make explicit reference in source files to the compiler being used (except, possibly, for disabling optimization with a pragma).
Performance
- Always use BLAS if there is a suitable routine. But don’t use BLAS if a vectorial expression or a single loop has to be replaced by several blas calls.
Scripts
- All scripts shall contain a “shebang” in the first line, ‘‘e.g.’’
/usr/bin/env bash
,/usr/bin/env python
, etc.
C code
- C code must be C99 compliant.
- You can use the inline and restrict modifiers; autoconf handles this in case they are not supported by the compiler.
- Declare non-modified arguments as const.
C++ code
- Only subset of C++ 11 is allowed, as long as it compiles with the IBM compiler. So avoid C++ 11 features, if possible.
- The use of header only code is encouraged.
Makefiles
- Lists of source files should be sorted alphabetically.
- An object file (.o ) should have dependencies on the main F90 source file it comes from, any F90 source files included by that one, and any object files in the same directory whose modules it uses.
- All parts of the code must build correctly with parallel make (make -j ) with any number of threads. If that fails, there is a mistake, usually missing dependencies.
- Every target that is not the name of a file should be in the
.PHONY
list.
Other
- Use spaces around operators like
<nowiki>=</nowiki>
,+
, etc. - Do not declare variables as pointer unless really necessary.
- Test files (.test
) should start with this line to enable proper syntax highlighting:
-*- coding: utf-8 mode: shell-script -*-
- Vector expressions should specify explicit bounds.
- (put my scripting stuff on wiki) – perl.pl etc.
- rules for m4
- Do not put profiling calls inside OpenMP regions.
Editor Configuration
To configure your editor to use our indentation convention:
- emacs : in menus, select Options->Advanced(Customize)->Emacs->Programming->Language. Click on F90, then on Indent. Now set all the numeric values here to 2, and Save and Done. Equivalently, paste the following lines to your ~/.emacs
file:
The last line also suppresses the warning emacs will give you when you open a symbolically linked file.(custom-set-variables '(f90-continuation-indent 2) '(f90-do-indent 2) '(f90-if-indent 2) '(f90-type-indent 2) '(vc-follow-symlinks nil)) (setq c-default-style "linux" c-basic-offset 2)
- vi : paste the following lines to your ~/.vimrc
file:
:let fortran_free_source=1 au BufNewFile,BufRead *.f90p,*.fp set filetype=fortran au filetype fortran set expandtab softtabstop=2 shiftwidth=2 autoindent set backspace=2
References
- http://xkcd.com/844/
- http://xkcd.com/292/
- http://xkcd.com/974/
- http://www.etsf.eu/system/files/etsf-coding-standards-1.3.1.pdf
- http://www.nature.com/news/2010/101013/full/467775a.html
- http://fortran90.org/src/best-practices.html
- http://www.kernel.org/doc/Documentation/CodingStyle