I looked back over the code, and the "state information" abstraction was
driven by the needs of the averaging and rmsd code for mdout, executed by
the master in runmd. I would have definitely preferred a structure or
record with reasonable names. I DO tend to use rather short abbreviations
in pmemd to map some set of global variables back to a given module, just to
help disambiguate things a bit, and more clearly identify things as global
(I actually use gbl_ all over the place for this purpose; it is sort of
stone ax, but much better than nothing). Unfortunately, I have yet to get
an entirely consistent naming convention; I am very much still living with
code that is "in migration" from the sander structure, and in fits and
starts I will clean up a bit. I would note that the biggest warning problem
I have with the fortran compilers is that the names I use are too long for
the compiler (kind of dumb to have these limits in this day and age). So I
understand the advantages of clearer names within limits (here, the "si_"
prefix was chosen to be short because I knew it would be showing up as an
array index all over the place, and I wanted the energy name to be the
descriptive part). A lot of s/w engineering is a matter of taste and
religion; I agree in general that clarity is a really really good thing,
though :-) I do think that get()/set() routines here would be overkill,
though, and I am not aware of an inlining standard for f90/95 (maybe I
missed something, but I did just drag down the iso/ansi reference). For
c++, this is a no-brainer, given that you have a situation where you want to
limit the modification scope.
Best Regards - Bob
----- Original Message -----
From: "Scott Brozell" <sbrozell.scripps.edu>
To: <amber-developers.scripps.edu>
Sent: Sunday, November 23, 2008 3:32 PM
Subject: RE: amber-developers: ene and ener array mess in sander
> Hi,
>
> On Sat, 22 Nov 2008, Ross Walker wrote:
>
>> > > Does anyone actually understand how the crazy ene / ener mess in
>> > > sander
>> > > actually works?
>
> Doubtful.
>
>> > There is a set of comments starting at line 1099 of force.f which might
>> > be
>> > a start some real code comments.
>>
>> Yeah I looked at them but they aren't complete. The main issue is being
>> able
>> to extend the array (add some extra terms on the end) without completely
>> breaking everything at the moment. The problem is that the size of the
>> array
>> is hand coded in so any places that it is a disaster to update it. I
>> might
>> try creating some kind of module and placing it in there so at least the
>> array size definition and all the comments etc will be in the same place.
>> I
>> guess this will need a day with door locked and the coffee setup to feed
>> intravenously :-).
>
> I also faced this mess in early 2004. My excuse for following the
> "path of least resistance" was my eminent departure.
> At the time i thought that the best cleanup would be an energy module
> of get and set routines. The main advantages of get/set seemed to be
> an improved chance of correctness and easier debugging:
> these routines would be a good spot for detailed verification
> with special test cases, tracing, and breakpoints.
> I also thought get/set might be good locations for investigating race
> conditions which i thought we might be having with psander.
>
> The main disadvantage of get/set seemed to be the anti-computer-science
> bent of some in the amber community. I figured that they wouldnt be
> convinced by a simple statement that any decent compiler would inline
> away those get/set routines. However, consistently chosen get/set names
> could be easily globally replaced into array references.
> (And globally re-replaced if the debugging need arised.)
>
> On Sat, 22 Nov 2008, Robert Duke wrote:
>> I do think my "state
>> information" abstraction is good - it is what you are really dealing
>> with -
>> ene's, pressure, temp, vol, etc. I looked back at the code, and what I
>> really
>> do is parameterize offsets into a "state information array" that is used
>> in
>> all implementations. Then for each of the electrostatics models
>> (currently
>> just pme and gb for pmemd), I create a "potential energy record (or
>> structure)" that clearly names everything associated with the given
>> electrostatics model.
>
> Clearly, clear naming is in the eye of the beholder:
> "si" is way to cryptic for a global variable name.
> Words, words, words: state_info is readable and only 10 chars.
> But Bob's state_info_mod is an example of the non get/set path.
>
>
>> > I believe the use of an offset of 23 goes back to Chandra Singh(!), but
>> > it
>> > might be even older (i.e. Paul Weiner).
>>
>> Ughh...
>
> The Joy of Scientific Programming ...
>
>> Oh well, I'll see how I get on. I might just have to take Bob's advice
>> and
>> gut the thing. It is certainly in need of some "rm " optimization.
>
> I second the rewrite approach with any reasonable implementation.
>
> Scott
>
>
Received on Fri Dec 05 2008 - 16:30:06 PST
