NOAA-PMEL / PyFerret

The PyFerret program and Python module from NOAA/PMEL
https://ferret.pmel.noaa.gov/Ferret/
The Unlicense
58 stars 22 forks source link

Documenting code cleanup, resolving compiler warnings #130

Open ACManke opened 1 year ago

ACManke commented 1 year ago

This is a list of steps for resolving compiler warnings. The Fortran warnings are:

Warning: Array reference at (1) out of bounds (2 > 1) in loop beginning at (2)

Warning: Initialization string at (1) was truncated to fit the variable (1/2)

Warning: Legacy Extension: Label at (1) is not in the same block as the GOTO statement at (2)

Warning: Branch at (1) may result in an infinite loop

Warning: Deleted feature: ASSIGN statement at (1)

Warning: Deleted feature: Assigned GOTO statement at (1)

Warning: Fortran 2018 deleted feature: DO termination statement which is not END DO or CONTINUE with label

Warning: Fortran 2018 deleted feature: Shared DO termination label

Warning: Fortran 2018 deleted feature: Arithmetic IF statement
ACManke commented 1 year ago

First, I notice that a number of routines that trigger these warnings are never called in the current code base. A check of this lets us remove a number of files. A list of these is attached. It includes a couple of cases where the file name does not match the subroutine contained in the files. I went ahead and renamed these:

/mem/mr_num_dims.F renamed file to match subroutine name, was mvar_num_dims.F fer/utl/dsg_coords_updn.F renamed file to match subroutine name, was dsg_coord_updn.F

cleanup_2022_removed_files.txt

ACManke commented 1 year ago

Of the files that are called, "Warning: Array reference at (1) out of bounds" occurs only in fer/efi/zgrid_laplace.F. This routine also triggers a number of other warnings, so a pretty complete rewrite is done.

ACManke commented 1 year ago

"Warning: Initialization string at (1) was truncated to fit the variable" is pretty obvious. For example, the strings that contain command qualifiers are declared as CHARACTER*8 but qualifiers such as /PRECISION are defined. Just shorten these strings. The result that the user sees will be unchanged.

ACManke commented 1 year ago

"Warning: Legacy Extension: Label at (1) is not in the same block as the GOTO statement at (2)"

occurs in ppl/plot/labdrw.F

This is a logic error. Luckily, a small change fixes this with no change to the actual logic that the routine uses.

ACManke commented 1 year ago

"Warning: Branch at (1) may result in an infinite loop"

This occurs in ppl/plotlib/habis.F and ppl/symlib/charin.F

I have changed the logic so the READ statements in these routines will not go into a possible infinite loop

ACManke commented 1 year ago

"Warning: Deleted feature: ASSIGN statement at (1)"

This is an old construct that let programmers do "if-then-else-else-else-- endif" operations, before that syntax was available.

For instance, in ez_count_dset.F, the commands to skip records at the start of a file previously were:

*   set up ASSIGN statements (for GOTO variable statements)
    IF ( unformatted ) THEN
      ASSIGN 400 TO skip_in         ! Goes to UNFORMATTED read
      ASSIGN 500 TO read_in         ! Goes to UNFORMATTED read
    ELSEIF ( in_format(:4) .EQ. 'FREE' ) THEN
      ASSIGN 450 TO skip_in         ! Goes to FORMATTED read
      ASSIGN 610 TO read_in         ! Goes to FORMATTED read
    ELSEIF ( stream ) THEN          ! "STREAM" direct access binary
      ASSIGN 499 TO skip_in
      ASSIGN 620 TO read_in
      irec = sf_skip(sfcnt) + 1     ! /SKIP=words
    ELSE
      ASSIGN 450 TO skip_in         ! Goes to FORMATTED read
      ASSIGN 600 TO read_in         ! Goes to FORMATTED read
    ENDIF
...

* read "SKIP" records and initialize records read counter
    GOTO skip_in
  400   DO 425 scnt = 1,sf_skip(sfcnt)
  425   READ (lunit,END=9200,ERR=9000)
    GOTO 499
  450   DO 475 scnt = 1,sf_skip(sfcnt)
  475   READ (lunit,'(1X)',END=9200,ERR=9000)
  499   nout = 0        ! is number of values (per var) read so far

This becomes:

* read "SKIP" records and initialize records read counter

    IF ( unformatted ) THEN

       DO scnt = 1,sf_skip(sfcnt)
          READ (lunit,END=9200,ERR=9000)
       ENDDO

    ELSEIF ( in_format(:4) .EQ. 'FREE' ) THEN

       DO scnt = 1,sf_skip(sfcnt)
          READ (lunit,'(1X)',END=9200,ERR=9000)
       ENDDO

    ELSEIF ( stream ) THEN          ! "STREAM" direct access binary
       irec = sf_skip(sfcnt) + 1     ! /SKIP=words
       nout = 0     ! is number of values (per var) read so far

    ELSE

       DO scnt = 1,sf_skip(sfcnt)
          READ (lunit,'(1X)',END=9200,ERR=9000)
       ENDDO

    ENDIF 

    nout = 0
ACManke commented 1 year ago

Some further cleanup:

The main PyFerret Make process includes system-specific files called platform_specific.mk.system to customize details of the build. These include locations of libraries, specifications of compiler flags, and a section called "MYDEFINES". This last section is identical in all of the platform_specific files currently in the code base. These defines are flags that are included in the Fortran source code, to accommodate things that need to be different from one system to another. These differences are for things like differences in the keywords used for opening files, or the syntax for defining some of the data types. Once Linux became the standard, most or all of these are unnecessary.

Here's a common one:

#ifdef sun
        BYTE      fhol(slen)    ! c-type Hollerith string buffer
#else
        INTEGER*1 fhol(slen)    ! c-type Hollerith string buffer
#endif

There are also a few that seem to have come from developers wanting to do something for a time to make debugging easier while development was underway, but then never coming back to clean it up.

All of these can be removed from the source code, and the section MYDEFINES can be removed from the platform_specific files.

ACManke commented 1 year ago

The edits to resolve ifdef's are all finished and the code checked in on my fork.

This is the end of the broad edits I think need to be done to bring the Fortran side of PyFerret up to date with compilers, up to gfortran/gcc v 10. The compiler flags should be looked at, at some point, but this finishes the code changes. I'll close this issue. I'll make sure there's an up-to-date pull request.