Riccardo Giannitrapani made the presentation to the reviewers [Heather Kelly (chair), Richard Dubois, Traudl Hansl-Kozanecka, Leon Rochester] and a large number of kibitzers.
Riccardo presented a comprehensive introduction to the package and a summary of current status.
One important aspect of the design of G4Generator is a GLAST-specific stripped-down version of the G4 RunManager class. Something like this is necessary since we want Gaudi to be in the driver's seat rather than G4, however it will require some care on our part to keep up to date as we move to new releases of G4. Richard asked whether the G4 people plan to make RunManager an abstract class so we can just override it rather than making an alternate version. Riccardo had already raised this question with them; they agree it is a reasonable idea, but implementing it has no priority just now since it is not of interest to "normal" G4 users. Richard also suggested that a summary of differences (and reasons for them) between the standard G4 RunManager and our version be kept in some suitable place.
One of the G4Generator algorithm properties is UIcommands, which allows a user to supply arbitrary G4 commands; however not all such commands are truly supported in our streamlined implementation. Riccardo promised to document which commands may be used.
Riccardo wondered aloud how much detail should be included in mainpage.h. He also mentioned the mild inconvenience associated with the way release.notes is referenced from the mainpage (it takes two mouse clicks to get to it). Leon may have some words of wisdom on the latter point.
There is a G4Generator test program which, up to now, has been maintained by Toby. It probably needs to more systematically exercise all relevant G4 features. Also it should be renamed to be in accord with the standard naming convention (packageName_test.exe).
Heather suggested perhaps including some images from the intro talk in the Doxygen documentation (e.g., could reference them from mainpage). Alternatively, could just include a link to the whole talk. Leon said that although it is possible to put images in CVS so that they can be part of the Doxygen doc, they take special hand-holding. On the other hand, putting them in CVS makes it easy to associate the correct version of an image with a particular release, whereas a link to a web document not under version control will tend to get out of sync.
Riccardo noted that there tends to be significant overlap (though certainly not identity) between information to be provided in header files in Doxygen comments and the regular C++ comments recommended for implementation files. Would it be possible to automate this?
We looked at Doxygen documentation and the actual code for a few classes: G4Generator, RunManager, DetectorConstruction and PosDetectorManager.
G4Generator: Heather suggested providing examples of plausible values for jobOptions parameters.
RunManager: Riccardo remarked that this class should probably be stripped down somewhat further. Richard suggested adding a link to documentation for the G4 version of RunManager to make it easier to compare.
Most of the other comments were less specific to the particular class under discussion.
Techniques useful for debugging, such as included alternate
versions of code within #if 0 ... #endif blocks, should normally be removed
before a "serious" tag.
What is our convention for "get" methods? Do we include the "get" in the name or not? General feeling was that it is recommended to include it. It can become tedious while writing code involving such methods, but probably pays off when reading it.
In situations like this (changing names to meet standards) do we want to keep the old version around for compatibility? (Richard) Generally not; the clutter it creates is too unpleasant. Better to make a clean break and change package version to reflect the change to the interface. (Leon) we need to document rules for version numbering.
There is currently a mixture of output forms used for messages: real Gaudi messages and output to std::cout. Should be using Gaudi techniques everywhere. Takes a little extra effort to do this from classes which are not themselves Gaudi algorithms or services.
Error handling is not always adequate. Would be helpful if somewhere we (not sure who the "we" is in this case) specified what is considered adequate!
(Richard) As currently configured, Doxygen class documentation has a link to the (cxx) source, but only towards the bottom. Would it be possible to make it appear near the top? (Heather) Probably, if we go to the trouble of making a GLAST-specific Doxygen style sheet.
Future reviewees will be pleased to know that Riccardo's coding rules checker is nearly ready for release.
Action Items
Back to DocTF Home Back to GLAST Software Home
J. Bogart Last Modified: 04-Aug-2004 15:59:25 -0700