Code standards

[JessicaMeixner-NOAA]

Here are a list of proposed code standards for WW3:

• No blank spaces at the end of lines
• Spacing/alignment of if/do/etc to be 2 or 4 spaces
• End statements (ie, endif) should align with the beginning (ie, if)
• Continue with first column of code is line 7
• When using modules, use ‘ONLY’
• No tabs
• Do not add computed GOTOs
• Limit additional GOTOs
• Define any new variable added
• Once doxygen is added (See issue Add Doxygen documentation support #547), must add doxygen updates
• 72 character line length maximum for code

Recommendations:

• good comments of code
• File naming convention:
  -- ww3_* for programs
  -- w3* for shared subroutines and modules
  -- wm* for multi- related shared subroutines and modules

[MatthewMasarik-NOAA]

I think it's good to have a set of standards for WW3, and I like all the ones so far proposed. I had one comment on the spacing/alignment. I have a slight preference towards 4 spaces for clarity, though would be fine with 2 otherwise.

[JessicaMeixner-NOAA]

I think currently in WW3, 4 spaces are used more often than 2. I don't have a strong preference either way.

[ukmo-ccbunney]

There are some routines that have quite deep nested if constructs - in these cases 2 spaces is a better choice to avoid excessive line wrapping. However, I don't have a particularly strong opinion...

[ardhuin]

Sorry but 2 spaces was the old standard (look at version 3.14 for exemple) ... and 4 will waste a lot of space!

[ukmo-ccbunney]

Tagging @UKMO-lsampson as he may have some recommendations regarding code standards off the back of the OpenACC work he has been doing.

[ukmo-ccbunney]

Do we want to set a line length limit?

[ukmo-ccbunney]

Do you want to keep to traditional Fortran line lengths (72 chars)?
My Vim syntax highlighting likes that, but it does seem a little short sometimes with modern F90 (usually when you have long strings)

[MatthewMasarik-NOAA]

I'm also familiar with 80 chars and think some modern widths are much higher ~120. I'll defer to more senior developers, but lean towards the shorter end, 72, 80 for the reason that it would maintain consistency with most of the WW3 code I've seen so far.

[JessicaMeixner-NOAA]

I agree it's kind of short, but also that it's annoying when vim doesn't highlight correctly. 72 works for me.

[JessicaMeixner-NOAA]

I added it to the list above

[MatthewMasarik-NOAA]

Correct syntax highlighting makes me happy :)

[DeniseWorthen]

As part of the work @mvertens and I have been doing regarding ifdef removal (discussion #763), we've created this list of best practices for discussion.

Best coding practices

• Implicit none should only be declared at the top of the module
• ALL use statements should have only clauses
• Pointer arrays in derived types should be initialized to null()
• Pointer arrays that are module variables should be initialized to null()
• The use of goto statements should be eliminated where possible
• The use of flush() should be eliminated (it no longer does anything in modern fortran compilers)
• Subroutine calls that have optional arguments should have keyword/value pairs for the optional arguments
• Local variables that have assignments should not use "save" statements- they are stored on the heap
• Fortran intrinsic procedure names should not be used as variable names (eg. using "write" or "status" as a variable name)
• Hardwired values should be avoided. Using a local variable allows the value to be changed in a single location rather
  than many locations. For example, memunit = iaproc+30000; write(memunit.....) preferred to write(IAPROC+30000....
• Character intent(in) arrays should inherit the length from the calling routine. CHARACTER(len=*), INTENT(IN) :: FEXT not CHARACTER, INTENT(IN) :: FEXT*(*)
• String intrinsics should be used where possible file = trim(fext) not ilen = len_trim(fext); file = fext(:ilen)
• F90 newunit should be used when possible open (newunit=
• Free Format F90 style should be used. Continuation marks are at the end of the line; the first 6 spaces on any line are not reserved
• Line length should be in range 80-100, to accomodate issues of screen real estate as well as printing. Shorter is generally preferable as long as readability is preserved. No line can be longer than 120.
• No trailing whitespace

Indent/Comment structure

The python script developed by @mvertens to automatically replace #ifdefs with logical flags also implements standard fortran90 indent and comment structure via emacs. This function allows f90 program control to be indented and commented in a uniform manner. This produces standard F90 indenting and commenting as

module mod1

  use module A, only : var1

  ! module default
  implicit none

  public

contains

  ! this describes the subroutine
  subroutine sr1()

    ! this is a use statement
    use module B, only : somevariable

    ! these are subroutine arguments
    integer, intent(in)  :: invar
    integer, intent(out) :: outvar
    logical, intent(in), optional :: flag

    ! these are local variables
    integer :: jindex
    real :: var1

    ! this is a do loop comment
    do j = 1,jmax
       ! this is an interior comment
       if (somecondition) then
       end if
    end do

  end subroutine sr1
end module mod1

[MathieuDutSik]

While I agree about the need to have better standards of writing,
there are several things to consider:

• I do not like that some rules like GOTO are only for additional code. I am not a fan of GOTO and use it extremely rarely, but sometimes they may have their place.
• Some rules like at most 72 characters may have sense but there could be cases where longer characters make sense.

Some rules like "good comments" are very good but it should be precise. I would propose:

• When implementing numerical scheme for solving equation or for some source terms, I think putting a reference to the paper is very important.
• If some complicated code took some time to the author to do, then he ought to write some explanation about it and why things are like that and why other approaches was bad.

Further thoughts:

• I agree that code standards are a good thing but too strict enforcement is maybe a bad idea.
• For other languages, there are automatic formaters like clang-format (for C++) or black (for Python). There should be something like that for fortran.
• Code standards can be unforced progressively instead of at once. This is a better approach than creating mayhem.
• Code standards can be detected on Github with github actions.

[JessicaMeixner-NOAA]

@MathieuDutSik thank you for your comments!

We are required by operations to minimize the use of GOTOs and not add to the total number of GOTOs for subsequent operational implementations. So while we do not have to remove every existing GOTO, we should likely think of another way to handle errors so that we are not adding more GOTOs. At this time, we do request that we not add more GOTOs in the code due to these rules. Computed GOTOs have been removed and will not be accepted in any subsequent PR in the code.

In terms of number of characters, perhaps we should look to see where we are with WW3 in terms of line numbers and pick a line length that makes sense for existing code, with a goal of minimizing it to XX characters (where XX=72 or other agreed upon number)?

I agree that moving from subjective to more precise standards is better (albeit hard at times). Hopefully we can find some other community codes that could provide examples for more precise standards or by providing more examples such as the one you gave by providing references where there is one to exapand one what precisely do we mean by "good code comments".

I know others (apologies can't remember who) has suggested we should try to have our code standards enforceable by github actions. If you know of any github actions that check for particular code standards or automatic formatters (I believe @mvertens has used some emacs tools), please pass them along!

I tend to agree that smaller changes are more manageable and will request that perhaps getting the code up to whatever agreed upon new standard is done in an a way in which 1 major change per PR as is typically requested. That being said, some of these changes will be impactful and unavoidable. If we must make these impactful changes (similar to changing from switches to ifdefs), what are your thoughts on having them all go in quickly versus being spread out over a longer time? Another thing we can possibly do is announce that any big change is coming and encourage any work in progress to be submitted for PR before a certain date as to minimize the impact as much as possible for developers, similar to what we did for the switches -> ifdefs transition.

Lastly, you made a comment on another thread about ifdefs and their impact on runtime. This is also a huge concern of mine that will have to be considered before implementing. You also mentioned offering to cleanup any DEBUG* if def comments that were no longer needed and it would be great if you could take a look at all of the ifdefs DEBUG* to see what is really needed versus what was only needed for a one-off case in hopes of greatly reducing the number of debug statements. Thank you so much for offering to do this, it would be a huge help!

[MathieuDutSik]

See https://github.com/github/super-linter
for the code standard check on the code.

[aronroland]

Hi everybody,

i start small but I will expand slowly ...

72 line length does not follow the F90 standard which is 132. Moreover this makes the code lengthy, hard to read and is something from the time when we had this https://de.wikipedia.org/wiki/Lochkarte. I do not understand the motivation for this. Rather I would write a python script to get rid of it. I think we do not want to go back to the year '77. I would like to understand the motivation having 72 line length. What is the reasoning for this, what is the benefit?

Cheers

Aron

[mvertens]

@aronroland - thank you for this comment. I could not agree more. Also - with emacs will automatically indent the entire file (with consistent indendentation for if-blocks, comments, etc) with this command -
emacs --batch <filename> -f mark-whole-buffer -f f90-indent-subprogram -f save-buffer
However, this will not ensure that you keep 72 characters as a maximum line length.
Using this command on each file flagged about 10 files as having #ifdefs that were not consistent with corresponding closing statements, etc. This was extremely useful for both Denise and I to then refactor and introduce new logicals to remove the flagged problems.

[ukmo-ccbunney]

I would be happy to see a longer line length! Although I will need to reconfigure Vim to avoid horrible syntax highlighting issues!

[aronroland]

Hello everybody,

when I was a decade ago at ECMWF i had the opportunity to talk to a young smart guy from the computational science division. It was exactly about the same topic and I have reported that during my visit in Washington approx. 5 years ago. Basically, it would be a bit different than what was suggested here and I like to move now to the "use only" topic. Especially, if #ifdef is contained in the "use only" part.

Now let me explain what I have picked and what I agree with. Now making extensive usage of "use" statements outside of the "driver" subroutines is not a good way to proceed. The reasons are the following:

1. It is not clear to the user where all this stuff comes from and whether it is really used or not. It is a cumbersome work to see where the variables come from and how they are defined or even if they are define or even at least initialized. I guess we all had this issue that occasionally variables are defined or not defined or even not initialized depending on the switches and the #ifdef.

2. For the compiler this is a mess. For the cache locality it is a predefined mess and things are loaded on the memory globally/locally even if they are maybe only used on one grid point. The compiler won't optimize properly maybe even shut down optimization if the certain calls are in innermost loops with complicated memory access. This depends on call segmentation and this should be kept simple for the sake of the above. To be clear, the opposite is the case and getting rid of the #ifdef will increase actually the complexity for the compiler.

3. When calling subroutines, especially the ones with the large "water head" e.g. w3srce or others. We have like many variables passed and used occasionally. For this kind of calls we need derived data structures in terms of w3sources%hs, w3sources%tm02, and so on. This significantly reduce the overhead of temporary arrays or memory duplication. It will a unified interface for all calls, whether it is advection, sources or output. Moreover, we will introduce cache locality by design, due to the fact that in the derived data-structures the certain values are near to each other.

I conclusion the usage of "use only" is not something that is very helpful, neither it is good for optimization or allows easy going code development, rather we should have "implicit none" without "use only" + "intent" statement in the subroutine header, which renders the various subroutines self contained.

Now we have opened here clearly a Pandora box, which is great but WW3 is now in a state of very rapid development and certain changes go from top to bottom. I am afraid that this will increase significantly the work load for all involved persons. Which is clearly worthwhile but which is maybe not now the time to go. My suggestion is that we further discuss and negotiate and do it slowly part by part. If something is really a hard problem for certain development we should seek & destroy but otherwise we should wisely design and coordinate.

I am opposed of such top level code changes that are not touching the essence of the problem and I think that we need to work now problem oriented rather than making large global changes that directly affect all ongoing developments and resulting in significant code changes.

I will comment more on the other points but all of them need a lot of attention a lot of thinking in the future as well as adjustment with the ongoing work. Nevertheless, it is super important to carry that forward and I strongly appreciate the action that have been take here.

Maybe a good start would be get rid of the #ifdefs for only the source terms but that would result then of the input files.

Otherwise the other coding standards suggested by Jessica are fine with me.

Cheers

Aron

[JessicaMeixner-NOAA]

@MathieuDutSik Thank you for your feedback. We appreciate your improvements in WW3 and we will be working towards improving the PR process for WW3, please feel free to start a new discussion thread or contact me offline to discuss improvements to the PR process.

[DeniseWorthen]

@aronroland I'd like to understand more about your belief we should not "use, only" (as in ``USE W3ADATMD, only : ....). I agree with your point that without use,only, "It is not clear to the user where all this stuff comes from and whether it is really used or not." The addition of the "only" clause does in fact serve as a type of documentation (what is used, what might change etc) and makes the user much more aware of the code organization etc.

But I'm not sure I follow your next point about the compiler having a harder time optimizing if you add the "only" clause. Is it really harder for a compiler to optimize a small set of variables that are actually used vs trying to optimize all public variables from some module?

Also, how does it make code development easier, other than allowing a developer to skip the step of adding the variables that they are actually using?

[aronroland]

fortran use statement

1. good if types are used, since types are not variables and have no influence on the runtime e.g. mesh geometry and so on ...

2. good if constants (integer, parameter :: PI) are used, because although they existed as variables at runtime, they can only be read, not written. Furthermore, they are independent of the respective model that is to be calculated. PI remains IP regardless of whether Worms or a unit test is calculated.

3. still ok under certain circumstances when using protected variables of a module. These variables can be read and written from within the module, only read from outside the module. This is useful e.g. for the variable of an NML. The module NML_m reads the NML file and writes the protected variables. Access to it from outside the module is only read-only. e.g.

   subroutine SWE()
   use NML_m
   implicit none

     FLOW = F ! << Compile time error
   

   end subroutine

This is okay for variables that only have one instance within the program (multithreading). And if the values <200b><200b>are not available at compile time, they are created at program init time and no longer change when the program is actually running. It can negatively affect the readability of the program, since variables are used that are not visible when the function is called, e.g. SWE(x,y,z). See next section.

4a) bad when describing variables. Because the flow of information within the program is no longer obvious.
The program cannot be read "from top to bottom", e.g.:

call A(x, y) ! Changes xy
... many lines of code ...
call B(x) ! Change y via use
... many lines of code ...
call C(x,y) ! ups y has changed?!

4b) Furthermore, the parallelization is difficult to impossible to trade. If multiple threads (or some other kind of parallelization
OMP, vectorize, etc.) of a process describes the same variable, it is very difficult to understand the behavior of the program even with locks/mutex. Since multiple threads change a variable at different times, and that's already a big challenge, it doesn't help if the variable is changed invisibly in multiple places in the code.

4c) If individual isolated subroutines are tested automatically in a test framework (possibly in parallel), the only useful interface is the subroutine's arguments. Here it can be clearly defined for each individual test what data goes into the function and compared with what the function returns in terms of data (via the arguments or return value).

e.g.

assert(f(1,2,3) == true)
assert(f(6,4,3) == true)
assert(f(-1,0,0) == false)

4d) This kind of protection against errors for parallelization/vectorization can be reached partly in Fortran using the keywords elemental and pure. A pure subroutine may not have any side effects, it may only open Write variables with intent(out/inout). The extension of that is elemental, then the subroutine may also expect which scalar values be called with arrays. Parallelization is then possible here.

The successful implementation of all this requires experience and a certain amount self-discipline, just throwing code down is not.

[ukmo-ccbunney]

Hi @aronroland

Thanks for your valuable insight in this.

However, I don't fully understand the arguments you make in points 4b and 4c above.

For (4b), surely the flow of the code is no less obvious whether you are using USE module or USE module, ONLY?
In both cases, variable y is modified explicit argument passing and use association in a different module/sub-routine.
At least with the USE ONLY statement, we can track that y is in scope in this module and is passed explicitly to something from this module. Is that not better.

For (4c), I don't follow how the addition of USE ONLY affects this. If a module is being tested in some sort of test harness, either the parameters are passed in explicitly via subroutine arguments, or via some sort of module use statement. For the latter, have this explicitly specified is again surely a good thing?

Apologies if I am missing some crucial point here!

[aronroland]

Hi @ukmo-ccbunney,

let me answer in a different way, maybe it can be wrapped up as follows.

1. We should do "use only" if we have r parameters.
2. We can have "use only" for protected variables e.g. mesh geometry, namelist or others
3. We should use subroutine interfaces by defining intent(inout) or intent(in) for variables that manipulated.
4. This also allows for some testing as given in 4c. The answer to your question is basically that u do know what has to be tested since u do not really know what i manipulated. Or let me say it is not obvious.

I do not say that we should "use" instead of "use only" in contrast I want it more restricted.

I hope this helps. Have fun! I think this needs further discussion and adjustment.

Cheers

Aron

[aronroland]

Hi All,

I like also to add that the PDLIB implementation and also all the DEBUG** code is not consolidated nor fully optimized. The problem is that we have always some problems, milestones, new developments, which hardly leave us time to consolidate and therefore I even more appreciate all this efforts, since this will reduce development time and errors during coding.

Maybe we can even agree on a coding marathon all together? Let's see during the user meeting. However, we need to have 1st some decent plan to carry on and define priorities. At least from my point of view.

Cheers

Aron

[ukmo-ccbunney]

One extra thing I would like to suggest is adding a comment on the ENDIF statement of vary large code blocks that specify what the IF condition is (e.g. the variable(s) being tested).

This will help with identifying the associated IF statement. I know this should be a lot easier once the indenting is sorted out, but there are still some very large IF blocks (e.g. in ww3_ounf) that make it hard to follow the code flow.

[mvertens]

Denise and have done exactly that in many of the currently protyped refactored routines.

[ukmo-ccbunney]

Great! The advantage of the C language curly brace is that it is easy to get editors to jump between code blocks... I've not managed to get Vim of VSCode to do this with Fortran, so comments on the ENDIFs would be nice! :)

[aronroland]

me too

[aliabdolali]

slides from Sep 6th Developer call

[JessicaMeixner-NOAA]

Hi Everyone - While this can always be a continual conversation, we are going to be taking into consideration comments made through Sunday, 9/12/22 and then the code managers will be taking into consideration all of the comments here and creating a list of code standard for WW3, which will be posted here 9/14/22. We appreciate your input and feedback and a big thanks to @DeniseWorthen and @mvertens for their presentation and efforts to move this conversation forward!

[stefanzieger]

Hi there, thank you for the presentations. I do like all of the proposed code changes. Particularly the use only option. Some function read/write directly from data arrays in other modules. Other will pass arrays through subroutines. From memory there should be a comment section in the header explaining if data is read or written. This may have been forgotten to update during code changes.

[aronroland]

I basically agree with the coding standards except some things, which I am opposing.

1. 72 character. I like 132
2. Fortran 90 continuation style
3. USE statements are ok, but not arbitrary ok, a statement with respect to this issue is given in the above discussion but this needs even more discussion. However, finally we need to come up with something decent. I have made some 1st suggestion in the discussion above.
4. Moreover, I like to stress that by changing from ww3 switches to cpp pragma we did not translate the code in such a way that we make use of the additional possibilities of the new pragma. Therefore expect that those and the future changes need then to be consolidated and the parts of code that includes those new features needs to be also consolidated in terms of the functionalism.
5. The other issue by initialization of all the allocated arrays/pointers. The #ifdef statements have not been consolidated and so a lot of memory was virtually allocated without intialization, which actually does then not manifest in the residual memory (only in the virtual). Now, by initializing all arrays/pointers this will automatically result into huge residual memory usage. To avoid this point 4 needs to be considered.

Otherwise I am more than happy about the outcome of this discussion.

Cheers

Aron

[aronroland]

Here is a real life example that illustrates the problem. The topics in 4b and 4c build on this.

In the best case, the programming error is detected at compile time and thus a faulty program does not arise in the first place.

Here is a code snippet from UnRunOff with an error that didn't noticeably change the results. (The fact that a program error does not significantly change the results is a catastrophe for the computer scientist, because then the error goes unnoticed for years or decades.
A bug that causes an immediate crash or obvious produces false results is very good!)

! There is at least one bug here
CALL EXCHANGE(UTOT)
CALL CALC_SPEED(UTOT, C, CNORM)
CALL applySinks(IT, ITSUB, DTINT, UTOT)

We have already passed the variable UTOT via subroutines argument and intent(in/inout). We also made sure that read-only variables are on the left in the arguments and write-only variables are on the right. It is immediately apparent that UTOT is written by EXCHANGE() and applySinks() and that C and CNORM are obvious written by CALC_SPEED(). With a little background knowledge, the first error is immediately apparent: UTOT is changed again after EXCHANGE(). So the other threads don't see the changes from applySinks(). Since it is very unlikely that the nodes of the sink will be ghost nodes contained, this programming error does not lead to incorrect results. (As a reminder: catastrophe.)

Now if applySinks() is called before EXCHANGE() to make the changes in UTOT visible to all threads, then you really messed up ;)

! Now its total wrong
CALL applySinks(IT, ITSUB, DTINT, UTOT)
CALL EXCHANGE(UTOT)
CALL CALC_SPEED(UTOT, C, CNORM)

The reason is: applySinks() uses the CNORM variable via USE,only! However, this is not obvious because it is not explicitly stated in the code. Now UTOT would be used from the current time step and CNORM from the previous time step.

Explicitly listing all variables as subroutine arguments read or written in applySinks() would have prevented this error (and a lot of time, money and frustration)!

We are not finish yet... The following code is also wrong:

! Still wrong
CALL CALC_SPEED(UTOT, C, CNORM)
CALL applySinks(IT, ITSUB, DTINT, CNORM, UTOT)
CALL EXCHANGE(UTOT)

Now applySinks() gets the variable CNORM that was previously calculated by CALC_SPEED().
But for the ghost nodes, C and CNORM are calculated incorrectly because EXCHANGE(UTOT) must be called first.
It's a cyclic dependency! Explicitly list all variables read/written by subroutines as arguments so that it is visible when reading the code,
which variable depends on which variable and subroutine helps enormously in recognizing this situation.

Here is the code with no more errors:

CALL applySinks(IT, ITSUB, DTINT, UTOT) ! Does not use CNORM, the speed is calculated on-the-fly from UTOT as needed for some nodes
CALL EXCHANGE(UTOT)
CALL CALC_SPEED(UTOT, C, CNORM)

Furthermore, the explicit listing of all variables that should only be read with intent(in) helps if one of these variables is written to.
This immediately leads to a compile-time error, and that's good because this type of error never occurs in a program that is used.
With USE,only it can only be recognized THAT the variable is used, but not whether it is changed and in what order of dependencies it is.
In terms of time and money, as many mechanisms as possible should be used. The best way to flag up errors is at compile time, or for the programmer to make them easier to find when reading through the code.

This is some example we experienced and that is quite actual this kind of stuff happens everywhere! I hope we could help and provide more insights in our concerns.

[MatthewMasarik-NOAA]

@aronroland Thank you for providing clarification on this issue, as well as thanks for continuing to engage with this discussion. It sounds to me that the use of USE and it's effect on those respective variables in different routines is the primary issue (vs. whether the ONLY modifier is used). Is that a fair paraphrasing? If so, I think this is a good point you bring up. The intent we had with this item is in line with your thinking, I believe, in that we are effectively saying, if you are going to employ USE, you should being doing it with USE, ONLY. The hope is that this helps to reduce some of the uncertainty you mention. To eliminate all the uncertainty would mean editing out ALL the current USE statements, which would be a truly massive re-write of the code base. What may be more realistic is having the preference toward use of in/out parameters for subroutines and functions, going forward. And if you are needing to employ USE, the standard is to add ONLY so both the author and user are at least more aware of what variables are currently in scope. This may be the best we can do considering how extensively USE occurs throughout the code base currently.

[aronroland]

Hi @MatthewMasarik-NOAA,

The use of use without only can be justified if we use e.g. only some fixed parameters but should be largely avoided.

As for the elimination of the given weaknesses it is clearly a big work but we are talking now about coding-standards and at a certain time they need to be fix for the future.

There are other points that are clearly more important issues, like memory usage that will also pop up soon. However, since we are now defining code standards we should be as clear as possible and clearly state what we want. This must not happen within days, I guess other developers and users will contribute as well when this is brought up in it's 1st version.

So I think it is fair to be pragmatic and just do as much it can be done at once.

The fine tuning of the code and the consolidation work is another story that needs much more consideration beyond the coding standards discussed here. I guess when removing all #ifdefs we will have all kind of fun!

[aliabdolali]

I second Aron's point of view.

[ukmo-ccbunney]

Thanks for the detailed explanations @aronroland - I understand your concerns now.

USE association is by definition very "USE"ful and simplifies subroutine calls by reducing the number of parameters you need to explicitly pass in, but I can see how this convenience can lead to unintended/unexpected side effects. I agree that in new code we should be paying closer attention to what is passed via the subroutine parameters (with the correct INTENTS of course) and what is USEd.

[JessicaMeixner-NOAA]

Hi Everyone - Thank you for contributing to the discussion about WW3 code standards! I want to say a special thank you to @DeniseWorthen for the presentation she gave with @mvertens last week and pushing us all towards better code! The EMC code managers (@MatthewMasarik-NOAA and myself) along with the community code managers (@alperaltuntas @ukmo-ccbunney, Erick Rogers, @mickaelaccensi, @thesser1, @sbrus89) created the list of WW3 code style standards we will use for WW3 moving forward based on this discussion. These standards are now on a wiki page and are copied below. Thank you everyone for your participation and contributions to WW3!

WW3 Code Style Standards

White Space

• No tabs
• No blank spaces at the end of lines
• Indention:
  • 2 spaces will be used for indentation
  • Open statements (ie if/do) should align with their closing statements (end)
  • The first 6 columns will no longer be reserved for spaces (ie no more fixed-format fortran)

Line Length

• The preference is to keep line length under 80 characters although up to 100 can be used by code or 120 by comment lines.

GOTOs:

• Do not add computed GOTOs
• Limit additional GOTOs

Modules

• Use “only” statements for any new module use statements
  • There will not be a concentrated effort to go back and update existing code from the code managers, updates doing this are welcomed from developers and any new code will be required to follow this.
• No implicit variables are allowed, i.e. use “implicit none”.

Documentation, Headers, Code Comments

• Moving forward the “WW3 headers are not required, while updates to remove existing headers is welcome, there will be no effort to remove all headers from the code. If you are removing an existing WW3 header, please take special care to ensure that relevant information is retained either via the doxygen header or in code comments.
• Doxygen will be used to document the code.
  • Any new subroutine, program or module must have doxygen. Information on WW3 doxygen requirements can be found in the WW3 Doxygen Templates.
  • Efforts are being made to include doxygen in all existing WW3 code.
• Please add good code comments. While “good” code comments is a subjective term, some examples are:
  • Provide references where appropriate.
  • Describe any code that might be interpreted to be “confusing”
  • Define variables

Capitalization

• The previous WW3 standard was that ‘permanent’ code used ALL CAPS and temporary code used all-lowercase. Moving forward, mixed-capitalization for new code will be allowed. There will be no effort to update existing code capitalization styles.

Naming Conventions

• Programs file names should start with ww3_
• Modules and subroutines file names should start with:
  • wm for multi- related shared subroutines and modules
  • w3 for all other files
• Variable names should be descriptive and are no longer required to meet the old 6 character fortran length. (Don’t forget to document their meaning through doxygen!)

General “Good Practice”

While no effort is being made to update existing code, moving forward the following items, which are considered ‘good practice’ should be followed:

• Pointer arrays in derived types should be initialized to null() unless immediately assigning it a value after declaration
• Pointer arrays that are module variables should be initialized to null()
• Fortran intrinsic procedure names should not be used as variable names (eg. using "write" or "status" as a variable name)
• Hardwired values should be avoided. Using a local variable allows the value to be changed in a single location rather
  than many locations
• Use utility routines when possible