RE: amber-developers: ene and ener array mess in sander

From: Scott Brozell <sbrozell.scripps.edu>
Date: Sun, 23 Nov 2008 12:32:45 -0800 (PST)

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:28:59 PST
Custom Search