Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor issues with SLAP #26

Open
billsacks opened this issue Dec 18, 2018 · 2 comments
Open

Minor issues with SLAP #26

billsacks opened this issue Dec 18, 2018 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@billsacks
Copy link
Member

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 billsacks added the enhancement New feature or request label Dec 18, 2018
@billsacks
Copy link
Member Author

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 billsacks changed the title Non-compliance with Fortran standard in SLAP Minor issues with SLAP May 14, 2019
@billsacks
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants