Re: [AMBER-Developers] Amber coding standards

From: Mark Williamson <mjw.sdsc.edu>
Date: Mon, 30 Mar 2009 06:35:08 +0100

Joe Krahn wrote:
>
> Also, the Amber9Forum section on "automatic vs allocated arrays" is not
> exactly right. First, the pointer array needs the SAVE attribute to
> ensure that the allocation is saved between calls. Second, it is better
> to use an allocatable array rather than a pointer array. Pointer arrays
> can sometime be much slower because pointers can be aliased, making some
> vectorizations unsafe. For a local pointer, the compiler may or may not
> be smart enough to assume no aliasing. Also, allocatable arrays are
> always initialized as unallocated, so there is no need for the
> 'first_call' flag. This might simplify some of the start-up code to
> avoid memory leaks with multiple calls to SANDER.


Hi Joe,

Relating to this point and eventually linking to the current gfortran -g
bug discussion in another other thread. When I started working on the
chamber code I hit an issue with allocatable arrays and gfortran 4.1.2.

Basically, gfortran 4.1.2 would fail compiling a piece of code like this:

   cat hello.f95
   program main

   contains

     subroutine foo(x)
     integer, allocatable :: x(:)
     end subroutine foo

   end program main

The error generated would be:

     integer, allocatable :: x(:)
                           1
   Error: In the selected standard, ALLOCATABLE attribute conflicts with
DUMMY attribute at (1)


These are the cvs logs that show where I "fixed" this issue in the
chamber code:


   [mjw:chamber]$ cvs log psfprm.f
   ---snip---
   revision 10.16
   date: 2009/01/23 20:05:44; author: mjw; state: Exp; lines: +35 -35
   MJW: Attempt to translate a F2003-ism to F95 to avoid the following
  error with gfortran < 4.2

    gfortran -c -O3 -fno-second-underscore -ffree-form -o psfprm.o
_psfprm.f
    In file _psfprm.f:2025

                       a0,a1,a2,a3,a4,a5,a6,a7,a8
                        1
    Error: ALLOCATABLE attribute conflicts with DUMMY attribute at (1)
--snip---


and the actual code change:

   [mjw:chamber]$ cvs diff -r 10.16 -r 10.15 psfprm.f
   Index: psfprm.f
   ===================================================================
   RCS file: /home/amber_cvs/cvsroot/amber11/src/chamber/psfprm.f,v
   retrieving revision 10.16
   retrieving revision 10.15
   diff -r10.16 -r10.15
   50c50
   < integer, pointer, dimension(:) :: iresid,ipres,iac,imove, &
   ---
> integer, allocatable, dimension(:) :: iresid,ipres,iac,imove, &
   59c59

--etc---

So, I changed the allocatable arrays to pointer arrays to avoid this
error. However, this is the converse to what is being said above.

Looking more into this bug and specifically this comment:

   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16136#c1

,I concluded at that point in time that this was a Fortran 2003-ism and
since SANDER is coded using the Fortran 95 standard, then this was
invalid code. Interestingly, if one uses a later version of gfortran
with -std=f95 then the same error will be obtained.

   mjw.ambersvn:/tmp$ gfortran -v
   Using built-in specs.
   Target: i486-linux-gnu
   Configured with: ../src/configure -v
--enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr
--enable-shared --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix --enable-nls
--with-gxx-include-dir=/usr/include/c++/4.2 --program-suffix=-4.2
--enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc
--enable-mpfr --enable-targets=all --enable-checking=release
--build=i486-linux-gnu --host=i486-linux-gnu --target=i486-linux-gnu
   Thread model: posix
   gcc version 4.2.4 (Ubuntu 4.2.4-1ubuntu3)

   mjw.ambersvn:/tmp$ gfortran hello.f95
   mjw.ambersvn:/tmp$ gfortran -std=f95 hello.f95
   hello.f95:6.27:

     integer, allocatable :: x(:)
                           1
   Error: In the selected standard, ALLOCATABLE attribute conflicts with
DUMMY attribute at (1)


However, Joe's email's prompted me to look into this again. Reading the
wikipedia entry for this:
   http://en.wikipedia.org/wiki/F95#Fortran_95

it mentions the supplement TR-15581
(http://www.nag.co.uk/nagware/np/doc/TR.asp) to Fortran 95 which
pertains to this. In a nutshell, this supplement permits the above code
within a F95+TR15581 standard, hence it's not a F2003-ism and my pointer
array changes are perhaps wrong.

gfortran implemented TR15581 in 4.2.0 and this is discussed here:
   http://gcc.gnu.org/ml/fortran/2008-05/msg00174.html


So, where am I going with this...? I see these options:

A) Request gcc.org to implement TR-15581 in gfortran 4.1 and release a
bug fix as gfortran 4.1.3 .

B) Revert my "fix" and modify AMBER's configure script to reject any
version of gfortran prior to 4.2.0 .

C) Keep the my current "fix" and allow versions of gfortran prior to
4.2.0 .

Option "A" I think is unlikely, plus, not all distributions may pick up
this fix. Option "C" seems silly since the compiler version is holding
back the benefits Joe states above and this problem is not seen in ifort.

I think option "B" would perhaps be the best thing to do, but this may
cause other issues, specifically with respect to packages and
distributions. I'll expand on this one:

Various linux distributions ship with a fixed version of a software
package. The distribution generally has a finite lifetime (say 10 years)
and over this time, maintainers will release bug fixes to these
packages, but will not update them to the latest version.

For example, Redhat Enterprise Linux 4 (RHEL4) ships with gfortran 4.1.2
and installing it is very easy:

   yum install gfortran

However, if I want to use gfortran 4.2.0, I will have to download the
source and compile it. I may also have to get and compile some other
libraries.... and soon it can get quite messy for the average user and
talking them through this on the mailing list is difficult, error prone
and generally frustrating.

Ideally, you want to work within the distribution's package management
system so that all these dependencies issues are resolved. This is the
key point of this email. The focus should be on the science and not the
nitty gritty of compiling package x which needs y which in turn depends
on the headers from package z.

Hence, you want to be able to say to a first time AMBER user something
like this:

1) Download this iso:
http://releases.ubuntu.com/releases/8.04/ubuntu-8.04.2-desktop-i386.iso
  , burn it and install the OS.

2) Install the following packages:
   sudo apt-get install gcc g++ libxt-dev libxext-dev flex gfortran tcsh
g77 libnetcdf-dev patch

3) Extract the amberX source

4) Apply the latest amberX bugfixes

5) Compile it.

We could create a checklist like this for all of the major distributions
and put these on the amber website.

Now, they can get on with the research and avoid the technical messing
around. Granted, this may not produce the fasted code (and we've not
touched MPI yet), but if the user wants to say get ifort and recompile,
then that can be the next stage, the point is, they are up and running
with minimal fuss. There is plenty to go wrong with an ifort
installation let alone the prospect of its own compiler bugs.

So, back to the choice, if "B" is selected, then this will determine
which of the distributions AMBER will compile on out of the box. Only
permitting AMBER to compile with gfortran >= 4.2.0 will essential rule
out RHEL4, but not RHEL5. It will also work with Ubuntu 8.04 ( gfortran
4.2.4), Ubuntu 8.10 (4.3.2) and Ubuntu 9.04 will ship with 4.3.3 . I'm
not sure what SUSE 10 or 11 ships with, but the underlying logic is the
same.

Any comments? (Apologies for the verbosity, but this has been on my mind
for a while)

Regards,

Mark

_______________________________________________
AMBER-Developers mailing list
AMBER-Developers.ambermd.org
http://lists.ambermd.org/mailman/listinfo/amber-developers
Received on Mon Mar 30 2009 - 01:12:09 PDT
Custom Search