I decided to start with analysing NASA modelE climate model because I could easily understand the build/configuration system and navigation my way around code easily enough. Some of the other models I have the source to seem a bit trickier. The analysis I'm about discuss isn't on the entire model source code, but only for one particular configuration of modules (an ocean-atmosphere coupled configuration though, so it includes many of the source modules).
In any case, I'll give you the goods upfront. I'll show you the big long summary of problems found forcheck found, but first let me explain the format. Here is an example item:
2x[ 84 I] no path to this statement
The "2x" means that this issue was found two times. 84 is the unique issue identifier that can be used to look up the issue in the forcheck documentation. I means that this issue is an informative type issue, as opposed to a W warning, or E error. The rest of the line contains a short description of the issue. Got it? Good.
Here's the big list:
1564x[344 I] implicit conversion of constant (expression) to higher accuracy
635x[681 I] not used
265x[675 I] named constant not used
144x[323 I] variable unreferenced
144x[699 I] implicit conversion of real or complex to integer
125x[ 94 E] syntax error
108x[109 I] lexical token contains non-significant blank(s)
107x[319 W] not locally allocated, specify SAVE in the module to retain data
96x[557 I] dummy argument not used
78x[345 I] implicit conversion to less accurate data type
65x[316 W] not locally defined, specify SAVE in the module to retain data
65x[665 I] eq.or ineq. comparison of floating point data with constant
38x[313 I] possibly no value assigned to this variable
35x[342 I] eq.or ineq. comparison of floating point data with zero constant
34x[644 I] none of the entities, imported from the module, is used
27x[124 I] statement label unreferenced
27x[315 I] redefined before referenced
27x[341 I] eq. or ineq. comparison of floating point data with integer
22x[125 I] format statement unreferenced
21x[ 1 I] (MESSAGE LIMIT REACHED FOR THIS STATEMENT OR ARGUMENT LIST)
21x[514 E] subroutine/function conflict
19x[530 W] possible recursive reference
18x[674 I] procedure, program unit, or entry not referenced
18x[598 E] actual array or character variable shorter than dummy
10x[340 I] equality or inequality comparison of floating point data
8x[325 I] input variable unreferenced
7x[347 I] non-optimal explicit type conversion
7x[565 E] number of arguments inconsistent with specification
6x[582 E] data-type length inconsistent with specification
6x[668 I] possibly undefined: dummy argument not in entry argument list
5x[312 E] no value assigned to this variable
5x[691 I] data-type length inconsistent with specification
4x[556 I] argument unreferenced in statement function
4x[621 I] input/output dummy argument (possibly) not (re)defined
4x[384 I] truncation of character variable (expression)
3x[383 I] truncation of character constant (expression)
3x[343 I] implicit conversion of complex to scalar
3x[454 I] possible recursive I/O attempt
3x[570 E] type inconsistent with specification
2x[568 E] type inconsistent with first occurrence
2x[573 E] data type inconsistent with specification
2x[ 84 I] no path to this statement
2x[651 I] already imported from module
2x[617 I] conditionally referenced argument is not defined
2x[214 E] not saved
2x[236 E] storage allocation conflict due to multiple equivalences
2x[700 E] object undefined
2x[307 E] variable not defined
1x[115 E] multiple definition of statement label, this one ignored
1x[145 I] implicit conversion of scalar to complex
1x[228 W] size of common block inconsistent with first declaration
1x[230 I] list of objects in named COMMON inconsistent with first declaration
1x[250 I] when referencing modules implicit typing is potentially risky
1x[667 E] undefined: dummy argument not in entry argument list
1x[676 I] none of the objects of the common block is used
1x[616 E] input or input/output argument is not defined
number of error messages: 200
number of warnings: 192
number of informative messages: 3415
I've only taken a peek at at a few of these issues in detail. Some of them are nonsense and can be disregarded right off the bat. For instance, the 125 syntax errors? Well, most of them come from the fact that the source files contain the cpp macros __FILE__ or __LINE__ and I haven't figured out yet how to make forcheck expand them (or I haven't worked out the ModelE Makefile magic to get cpp to do it instead).
Looking at the most frequent issues now. The most frequent is casting constant to a higher accuracy. Here's an example from the file QUESDEF.f:
frac1=+1.
where frac1 is defined as a REAL*8. I wouldn't think that casting unknowingly to a higher accuracy would pose much of a problem... but what do I know. Can anyone think of some examples where it would be?
The next most frequent issue is the "not used" issue. The issue here is that a variable has been declared but then never gets used before it goes out of scope. There are two examples of this in the file OCNFUNTAB.f, a module with lookup table functions. From my cursory look, it seems that many of the functions are similarly structured: both in purpose and in terms of documentation and layout. My guess is that in this specific case this evolved from copying an existing function to use as a template for new one, and forgetting to remove the unused declared variable. A code clone. This isn't always the case, of course. In another file with this issue, SNOW.f, it appears to be the result of commenting out code.
This issue is distinguished from the one that is two below it on the list, "variable unreferenced". The variable unreferenced issue refers specifically to when a variable is declared, and a value is set, but the variable is never accessed. This issue also occurs in SNOW.f, in this curious example:
ccc !!! ground properties should be passed as formal parameters !!!k_ground is used later in the function, but c_ground never is.
k_ground = 3.4d0 !/* W K-1 m */ /* --??? */
c_ground = 1.d5 !/* J m-3 K-1 */ /* -- ??? */
Next up is the "implicit conversion of a real or complex to integer". Here'r a few examples:
QUESDEF.f: prather_limits = 0.I, of course, have no idea whether these cases are unintentional, or what the effects of these casts are. I would think that it is generally dangerous to cast unintentionally to a less precise number...
OCNFUNTAB.f: JS=SS
RADIATION.f: JMO=1+JJDAYS/30.5D0
Let's look at one last issue, the cryptic (to me), "lexical token contains non-significant blank(s)". Forcheck describes by saying:
In a fixed format source form blanks are not significant. However, a blank in a name, literal constant, operator, or keyword might indicate a syntax error.Okay, I still don't get it.. but that's because I'm at all familiar with Fortran (yet). Here's one example from RADIATION.f:
DATA PRS1/ 1.013D 03,9.040D 02,8.050D 02,7.150D 02,6.330D 02,Each value, "1.013D 03" for example, is flagged as an instance of this issue. In this particular case these are just ways of writing down Double-precision numbers, but normally (I guess?) you wouldn't see the space, but instead a + or - sign. There might be a forcheck compiler option I can flip that will ignore this particular type of issue. I tried looking for other, possibly more significant instances of this issue. I found many that were "just" because of line continuations. That is, a line was intentionally wrapped and so this appeared as a space at the end of the line and the start of the next. Here are a two examples:
1 5.590D 02,4.920D 02,4.320D 02,3.780D 02,3.290D 02,2.860D 02,
2 2.470D 02,2.130D 02,1.820D 02,1.560D 02,1.320D 02,1.110D 02,
3 9.370D 01,7.890D 01,6.660D 01,5.650D 01,4.800D 01,4.090D 01,
4 3.500D 01,3.000D 01,2.570D 01,1.220D 01,6.000D 00,3.050D 00,
5 1.590D 00,8.540D-01,5.790D-02,3.000D-04/
SEAICE.f: IF (ROICE.gt.0. and. MSI2.lt.AC2OIM) thenI had no idea that fortran has such awful syntax for logical expressions. Yes, you read it correctly, the greater-than operator is .gt. and so on. In any case, in the above example in SEAICE.f, the fact that the and-operator is written as ". and." rather than ".and." is what raises this issue. In the ODIAG_PRT.f file, it is the fact that there is a space in the representation of the double-precision number.
ODIAG_PRT.f: SCALEO(LN_MFLX) = 1.D- 6 / DTS
This concludes my look at at the forcheck results for now. I should note a few things about how I configured forcheck to actually analyse the results. The most important thing to know is that forcheck can be configured to analyse the syntax according to the quirks of various commercial compilers. For this example I chose to use the Absoft Pro Fortran 90/95 V9 compiler emulation mode because the documentation for ModelE suggested this compiler works well. I had to slightly customise the compiler configuration to enable cpp preprocessing (off by default). I also had to configure forcheck with two cpp "defines" that specified the compiler and target architecture because there were a few places in the source code that had conditional compilation rules.
As mentioned, I didn't analyse the complete source code of the model. Not every module is used for every run configuration. I simply looked at one of the run configuration files that comes with the model, and configured forcheck to analyse only those modules that were specified there. For example, there is a module each for a variety of different resolution configuration: RES_M53.f, RES_M24T.f, etc., as well as several different versions of, what looks like, physics modules: CLOUDS.f and CLOUDS2.f for instance. The run configuration specifics only one resolution module and only one version of the clouds module and so that's what I analysed.
I point all of this out only to be explicit about the limitations of what I'm reporting here. This is just one slice through the code and, as you would expect, the static analysis report is often misleading. Nevertheless, I think this will make for some interesting starting points for discussion about code quality issues.
