ESCOMP / CISM

Community Ice Sheet Model
GNU Lesser General Public License v3.0
6 stars 11 forks source link

Minor issues with SLAP #26

Open billsacks opened 5 years ago

billsacks commented 5 years ago

I'm not sure if this is worth fixing (see #14), but @grnydawn points out:

I recently evaluated a fortran parser for my own work, and found several things that we may improve code quality of CESM in terms of Fortran standard conformance.

In "cism/source_cism/libglimmer-solve/SLAP/xersla.f" and other several files, there is a line similar to "format (15h error number =,i10)". By standard, a string literal should be wrapped by quotation mark, either single or double. But again, compiler generally accepts it. Right form is "format ("15h error number =",i10)".

Within SLAP, from a quick search, I see lines like this in xersla.f but not in other files: From git grep -n 'format *(':

xersla.f:289:   21          format ('(11x,21hin above message, i',i1,'=,i',i2,')   ')
xersla.f:295:   23          format ('(11x,21hin above message, r',i1,'=,e',
xersla.f:303:   30          format (15h error number =,i10)
xersla.f:383:   10       format (32h0          error message summary/
xersla.f:394:   40       format (41h0other errors not individually tabulated=,i10)

@grnydawn, did you see other instances of this?

billsacks commented 5 years ago

From @whlipscomb

For what it's worth, I'd like to continue including SLAP in the CISM build, at least for now, because it's handy to have a serial all-purpose matrix solver available. But I don't expect we'll ever upgrade it. If it wouldn't be too much trouble to hand-code a few fixes in format statements, that might be the best solution.

@Katetc I'm going ahead and assigning this to you, if that's okay with you. Not super critical, but it should be mainly a matter of making sure we've tracked down all problem statements, fixing them as per @grnydawn's suggestions, then testing the build via the test suite.

billsacks commented 5 years ago

Another issue was brought to my attention by @jedwards4b (who in turn got this from a colleague at IBM):

The XL Fortran compiler does not add an underscore by default, and what happened was caused by a name conflict involving "rand" :

cism/source_cism/libglimmer-solve/SLAP/xersla.f : defines rand(r)

the above rand(r) is used only in : cism/source_cism/libglimmer-solve/SLAP/dlapqc.f

Unfortunately, other software elements in MPI-IO need to use the rand() routine from the standard C library, but with IBM XL they get the Slatec rand(r) instead, which results in problems. Of course that would not happen if IBM XL Fortran would add an underscore to Fortran entry points, like everybody else ... the problem would go away with -qextname. However, I think it is best to use a more specific name in the Slatec libraries ... maybe slatec_rand(r) or slrand(r) in the two source files above.

@Katetc I think this is worth dealing with sooner rather than later, since it's causing problems and should be easy to resolve.