Hi,
For my $0.02 I think that Dave has made the right call.
The sources of some programs, like sander, are the result of a
large community effort. Even the added complexity and burden of
clearly written ifdef's is nontrivial from a utilitarian viewpoint.
The sources should be polished and bullet proof and that means that
the trash has to be eliminated.
Regarding debugging standards, the trace module in src/sander already
contains wrapper subroutines for common debugging tasks like logging
entry/exit from routines and printing the values of variables.
It even has a method to emit a table of data using optional arguments.
These can be all turned off or on via the preprocessor name TRACE
with the only ifdef being inside the Trace module.
It would be easy to change the on/off into a verbosity range.
Since the calls to the trace methods are always in the source files,
there are no issues of out of date code lingering in preprocessor ifdefs.
However, even Trace-ing can be an eyesore.
Search the sander sources for trace_ for example usage.
Here is an example:
! <compile=optimized>
#include "copyright.h"
#include "dprec.h"
#include "assert.h"
!+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
!+ trace test
program test
use trace
integer, parameter :: IMAX=3
integer, parameter :: JMAX=4
integer, parameter :: KMAX=2
integer i, j, k
_REAL_ data(IMAX,JMAX,KMAX)
call Trace_enter( 'my_subroutine' )
call Trace_note( " i j k data" )
do i = 1, IMAX
do j = 1, JMAX
do k = 1, KMAX
call Trace_table( i1=i, i2=j, i3=k, r4=data(i,j,k) )
enddo
enddo
enddo
call Trace_exit( 'my_subroutine' )
end
make AMBERBUILDFLAGS='-DTRACE' test
./test
which produces
Trace: 0:Enter my_subroutine at 182139192.58521 seconds
Trace: 0: i j k data
Trace: 0: 1 1 1 0.0000
Trace: 0: 1 1 2 0.0000
Trace: 0: 1 2 1 0.0000
Trace: 0: 1 2 2 0.0000
Trace: 0: 1 3 1 0.0000
Trace: 0: 1 3 2 0.0000
Trace: 0: 1 4 1 0.0000
Trace: 0: 1 4 2 0.0000
Trace: 0: 2 1 1 0.0000
Trace: 0: 2 1 2 0.0000
Trace: 0: 2 2 1 0.0000
Trace: 0: 2 2 2 0.0000
Trace: 0: 2 3 1 0.0000
Trace: 0: 2 3 2 0.0000
Trace: 0: 2 4 1 0.0000
Trace: 0: 2 4 2 0.0000
Trace: 0: 3 1 1 0.0000
Trace: 0: 3 1 2 0.0000
Trace: 0: 3 2 1 0.0000
Trace: 0: 3 2 2 0.0000
Trace: 0: 3 3 1 0.0000
Trace: 0: 3 3 2 0.0000
Trace: 0: 3 4 1 0.0000
Trace: 0: 3 4 2 0.0000
Trace: 0:Exit my_subroutine at 182139192.58688 seconds
This reminds me to cleanup trace.f so that the processor number (here 0)
is only emitted for parallel builds. I'll do that when amber10
is created. And add some more polish based on using a java
reimplementation
of this trace module.
Scott
On Thu, 9 Mar 2006, Robert Duke wrote:
> Okay, but I frequently comment out debugging-related writes, leaving
them in
> place should they be useful later (or sometimes I have an obvious ifdef,
> including DBG somewhere in the name). I would like to be able to retain
> such mechanisms, and consider them okay myself, as long as it is clear
in
> comments that the stuff supports debugging only (and as such, it IS
possible
> that they will be obsolete - but they should never be left activated in
the
> product anyway). We could probably develop some debugging standards for
> this sort of thing; trouble is the general fortran "this is a debug
line"
> convention turns all the debugging on, as would any specific define.
> Regards - Bob
>
> ----- Original Message -----
> From: "David A. Case" <case.scripps.edu>
> To: <amber-developers.scripps.edu>
> Sent: Thursday, March 09, 2006 3:20 PM
> Subject: amber-developers: Undocumented namelist variables
>
>
> > On Thu, Mar 09, 2006, David A. Case wrote:
> >
> >> From: Scott Brozell <sbrozell.scripps.edu>
> >
> >> In 8 there was an undocumented pb namelist variable nsaa.
> >> In 9 there is an undocumented pb namelist variable maxarc.
> >
> > This is a tough call, and I have not been consistent in the past. One
the
> > one hand, having undocumented options is attractive to developers.
But it
> > has turned out the users try them anyway, future developers are
confused
> > by them, and those options often become broken by future code changes,
> > because
> > they are not being tested.
> >
> > So the new policy is: no undocumented namelist variables. (Scott's
> > "retired"
> > varaibles are OK: they stop execution with an informative error
message.)
> > If you need local variables for your personal work, make a CVS branch.
> >
> > [As with everything, exceptions may be granted, but I think this is
the
> > correct general rule. A corollary is: don't leave "commented out" or
> > "ifdef-ed out" stuff in the code. Such stuff is as likely to be
confusing
> > as
> > helpful, and "turning it back on" may break things since the code has
> > evolved
> > since it was originally written.]
> >
> > We'll try to get the above rules implemented as much as possible by
the
> > time Amber 9 comes out.
> >
> > ...thx...dac
> >
>
>
Received on Wed Apr 05 2006 - 23:49:40 PDT
