-
Notifications
You must be signed in to change notification settings - Fork 630
Developer Commit Guidelines
As the FDS developer team grows it is becoming more and more necessary that we follow some ground rules that will allow us to continue to work together efficiently. The guidelines below closely align with this article on Best Practices for Scientific Computing.
Short answer: as often as possible! Commit early and often. Fail fast.
With that said, we obviously do not want to spend all day, everyday fixing errors caught by the continuous integration system. So, here are the guidelines for what we would like to see in a pull request (PR):
-
Your code should have verification (unit) tests. Here is a guide to setting up a verification test.
-
Your code should have documentation. You can find information on compiling the manuals here. Note that once a feature is documented users will expect it to be ready for use in the next release. Features that are in beta testing should be documented as such. In some special cases we may opt to maintain a separate document during development (for example, this is being done with Complex Geometry), but it is still important that other developers have access to the documentation.
-
Your code should compile without warnings in debug mode with Intel (if possible) and GNU compilers. You can find information on compiling FDS here.
-
Your PR should not contain merge commits. The main reason for this rule is that often what we see is that a PR with lots of merges will have mistakes in other areas. If we cannot have a reasonable level of confidence that a binary file has not been accidentally committed, then we cannot merge the PR. This means it is important to make your PRs reasonably small (just a few commits) and organized.
-
If a large set of commits is unavoidable, it is helpful to get in touch with us ahead of time. We can then make arrangements to go through the necessary testing of the topic branch before merging to the trunk.
Please note, there may be times, usually close to a minor release, where we will not accept major code or documentation changes.
- Stay up-to-date with the latest code. Generally, this means pulling changes from the central repository branch firemodels/master on a daily basis. For more instructions on how to setup remote tracking in Git see the Git Notes Getting Started wiki.
- Update the theory manual as necessary.
- Update the verification and/or validation guides as necessary.
- Update the users guide as necessary (hide developer hooks with comments, but do still include them!).
- Update release notes.
- IMPORTANT: Code changes should come with a test case, usually a verification test. Code modifications that do not have a test case have no guarantee of surviving in future development work. All test cases must be added to the guides. Either modify the script configuration file to process plots for the manuals or add your own script to the master script.
- Monitor the discussion forum for topics related to your areas of the code.
- Make use of the GitHub issues page to track progress of development work or bugs.
- A little passion doesn't hurt: We strive to make FDS the best fire code it can be. Our team is dedicated to this goal and works tirelessly in its pursuit.
- IMPLICIT NONE (enough said).
- Use ALL CAPS in F90 text files (readability and easy searching).
- Use 3 spaces (NO TABS!) for indention of loops, IF statements, etc. (readability, portability across text editors).
- Do not include excess white space. You can see the spaces with an editor like Sublime Text or Atom.
- Double space between subroutines and single space within subroutines (readability).
- Try to group all variables of the same TYPE in a single dimension statement. For example, if you have 10 REAL(EB), then put them on one line rather than 10 separate lines (reduce lines of code).
- Single space below major block comments (consistency in commenting).
- Use the Makefile that is in the repository for release and debug versions. Before committing new source: update your repository just prior to committing and compile in debug mode and make sure there are no errors or warnings.
- If an IF block, DO loop, or similar feature is more than a screen length of text, name the section. (helps keep track the start and end of long or complex code structures)
- Be explicit when using CYCLE or EXIT in a named DO loop or IF block. For example, use CYCLE LOOP_NAME.
- Use a named variable rather than a number, for example:
RHO_H2O = 1000._EB
rather than just1000._EB
(improves readability) - Use integers rather than strings for SELECT CASE, IF blocks, etc. (string comparisons are more costly)
- Use
>
,<
,>=
,<=
,==
, and/=
rather than.GT.
,.LT.
,.GE.
,.LE.
,.EQ.
, and.NE.
Put spaces before and after.AND.
and.OR.
but no space between.NOT.
and the logical variable. (improves readability) - Avoid the use of
==
and/=
for comparing REAL numbers. Rather thanIF (X == 0._EB)
useIF (ABS(X) < TWO_EPSILON_EB)
, and rather thanIF (X /= Y)
useIF(ABS(X-Y) < TWO_EPSILON_EB)
. - Explicitly type REAL numbers. Rather than
X = 0.
doX = 0._EB
(or_FB
if the appropriate precision is four byte). - Use descriptive variable names for improved code readability. For example,
EXTERNAL_WALL_CELLS
rather thanEWC
. - If you hardwire a number into the code, make it a PARAMETER and give some concise reason.
- If an input is added to a namelist definition, place it alphabetically. Seeing which inputs have not been documented in the User Manual is much easier when both the manual and the source is alphabetized.
- Do not use more than 132 characters per line. (line length limit)
- When using IF statements, put a space before and after the brackets, e.g.,
IF (statement) THEN
. (readability) -
ENDIF
andENDDO
each are one word with no spaces. (consistency) - Use
I0
to format integers for output if alignment does not matter. - When adding a new output
QUANTITY
, use unit# and not unit^# when defining theUNITS
, e.g. m2 and not m^2 for area. This will ensure thatSPATIAL_STATISTICS
works correctly. - Put yourself in the shoes of others who have to debug the code. Treat the code like you are writing prose: rewrite until it is clear. If someone can't glance at the code and understand what is happening, it needs to be rewritten.
It is helpful is we all use the same conventions for commit messages as this aids in searching the Git logs. Over the years the following have become standardized among the FDS developers:
- FDS Source
- FDS User Guide, FDS Tech Guide, FDS Validation Guide, FDS Verification Guide, Biliography
- FDS Validation, FDS Verification
- Matlab
- [others]
A couple of convenient ways to search the logs are by either grep
or looking at particular directory. For example, you could grep on FDS Source
commit tag like this
$ git log --oneline --no-merges --grep="FDS Source"
Or, you could list commits of files in the fds/Source
directory. Go to the top level of the rep and do
$ git log --oneline --no-merges Source
- The mathematical style of the documents should follow A. Thompson and B.N. Taylor. The NIST Guide for the Use of the International System of Units. NIST Special Publication 881, 2008.
- Remember to add new input parameters to the FDS User's Guide. If the parameter is not for public viewing yet, just put a comment character in front of it. This helps us keep track of inputs.
- Before committing a change to a manual, pdflatex it and make sure nothing is broken. You should be able to run the
make_guide.sh
script in the guide directory and get the message<guide> built successfully!
. If you don't, then firebot will get errors or warning messages that someone will have to clean up. - Do not commit the PDF version of a manual to the 'All PDFs' folder in the Repository until we do a release. Sometimes, input parameters get introduced in the guides before an actual release version of the code.
- Avoid HTML links in the manuals. They too often break.
- Use conventional (soft) wrapping! This means that there are no hard character returns embedded in your paragraphs. Currently, the FDS documentation is riddled with this problem and we are slowly getting around to cleaning it up. Please help us by reformatting any sections that you find yourself working on. (portability across text editors)
- Add any custom commands or latex variables to Bibliography/commoncommands.tex.
- Use \textbf, \texttt, etc., instead of \bf, \tt, etc. because the latter are deprecated.
- In captions for sections, figures, and tables make sure the corresponding entries in the Table of Contents, List of Figures, and List of Tables are only one line (no wrapping), and that the line does not abut the page number (there should be at least one dot between the line and the page number). If the full caption is too long, use an abbreviated caption in square brackets within the figure or table environment. There are plenty of examples to follow in the guides.
- Do not arbitrarily resize plots. Follow the examples to maintain consistency. Generally,
height=2.15 in
is preferred unless there is a special circumstance. Also, make sure single plots are centered.
- Read the wiki pages that describes the process of compiling the V&V Guides. Make sure that you can compile the Guides before making any changes.
- File naming convention: try not to use a hyphen (causes problems with tab completion in Cygwin).