Review of ifs/pppl module. By Lynda LoDestro Dear Fellow Library Committee Members: Enclosed is my review of the ifs/pppl transport module. I would like to thank the authors for their responsiveness and sustained willingness to make changes in what was probably a much more drawn out process than they had imagined. I would also like to remark that this first module review should not set a precedent, amounting to a deterrent, for the review process (though the final module serves as a very good example of a module): (1) the review, making practical use of our standards for the first time, showed up various problems with the standards themselves, which had to be worked out in parallel with the module review; (2) the authors agreed, in trying to meet Goals, not just Standards, to go beyond what was required for acceptance, and so provide a good example of things in this first reviewed entry in the Library. In writing the review below, I aimed to follow our chairman's instructions: > The report should indicate that the module adheres to standards, and > summarize where (in your view) improvements could be made, consistent > with modules library goals and standards, as well as any other comments > you deem relevant and appropriate. Regards, Lynda ------------------------------------------------------------- I have reviewed the ifs/pppl transport module and am happy to be able to strongly recommend that the committee accept it as the first entry into the NTCC Library. There are two areas which in my view could be improved; the authors are in disagreement with these points. Let me emphasize that the Standards as well as a substantial fraction of the Goals have been met, and I recommend approval of the module in its present form, while noting the issues for the record. The first is the nonconforming to the practice of I/O modularization (not presently a Goal, but under committee discussion). The module as it would be used in a transport code consists of a single fairly low-level (i.e., a diffusion coefficient) subroutine, yet it has (optional) writes scattered throughout, turned on, for example, when running the test program. I/O modularization would call for removing these statements to a higher level routine (at a minimum) or to an output module (as argued by some in earlier NTCC discussions). In either case, in the present submission this would simply mean putting the writes in the driver program or some other routine called by the driver. These write statements were not in the subroutine of Dorland, the original author, but were added by the present submitters. The second issue is in regard to documentation. The style employed by the authors is the source-code-embedded-in-a-tex-document system. Though it meets the Library Standards, in the reviewer's opinion this system puts an undo burden on both the user who wants to know precisely, concisely, and only about the usage; and on the person whose interest is in the physics or algorithms but not the implemetation. The authors have provided a table of contents to help the user (and the reviewer) through it, an improvement, but nevertheless, two items (in the Standards -- statement of bugs, and statement of i/o unit numbers employed) happened to lack entries in the table, and one had to resort to pattern searches to find any information. It is now time to vote. Please reply with an email enclosing your ballot (between the +++++ lines). +++++ NTC Modules Library Committee VOTE to approve ifs/pppl module (please choose one): YES NO ABSTAIN Module reviewer's vote: YES +++++ Appended is the form provided by Doug, derived from the latest version of the NTCC Library's Standards and Goals, with item by item checks. I've adopted a MET/NOT MET/UNTESTED/NA grade, with only occasional comments (NA = not applicable). ============================================================================= ============================================================================= ** GENERAL STANDARDS ** ============================================================================= Standard: Provide source code for each physics module or code. ----------------------------------------------------------------------------- MET ============================================================================= Standard: Provide test case(s) with driver program(s) with input and output data and their documentation. ----------------------------------------------------------------------------- MET ============================================================================= Standard: Provide script to compile and link (e.g., makefile). The script should make at least some provision for portability to multiple brands of UNIX (at minimum). Provide clear documentation (possibly in the README file) on how to use the script or makefile. ----------------------------------------------------------------------------- MET ============================================================================= Standard: Provide a README file giving (a) the name of the module and its authors, (b) the location and form of general module documentation, and (c) information (or pointer to more detailed documentation) enabling a user to build binaries from the source code. ----------------------------------------------------------------------------- MET ============================================================================= Standard: Provide documentation about how the module should be used, for example, whether the module needs to be initialized or used sequentially. Important usability issues, such as the existence of state information in COMMON or other static memory, which persists between calls, must be described. ----------------------------------------------------------------------------- I didn't catch explicit statements in the latter regard, but the input and output are very clearly described. Since it is just a single routine and there are no common blocks or save statements, one expects successive calls do work properly. ============================================================================= Standard: Eliminate graphics calls embedded in physics modules. ----------------------------------------------------------------------------- MET ============================================================================= Standard: The source code files (e.g. *.f, *.c, or *.cpp files) should be submitted rather than requiring extraction from another file. ----------------------------------------------------------------------------- MET ============================================================================= Standard: Authors may upgrade their modules with approval of the current chairperson of the NTCC modules committee. If the upgrade be subject to a full review. ----------------------------------------------------------------------------- NA ============================================================================= Goal: Multi-platform portability (code should run on different computers). ----------------------------------------------------------------------------- MET (The makefile currently handles three workstation unix operating systems, and is set-up to be easily extendable to other unix platforms.) ============================================================================= Goal: Provide error checking (but not stops). ----------------------------------------------------------------------------- MET ============================================================================= Goal: Portability (code should run in different environments, e.g. different operating systems). ----------------------------------------------------------------------------- UNTESTED (THIS GOAL SHOULD BE REWORDED. DIFFERENT PLATFORMS OF COURSE HAVE DIFFERENT OS'S. WHAT IS MEANT, IN DISTINCTION TO THE MULTI-PLATFORM GOAL ABOVE, IS: NOT JUST UNIX, or just macs, or... RIGHT?) ============================================================================= Goal: Minimize external dependencies that cost money (i.e. avoid using expensive proprietary licenses). ----------------------------------------------------------------------------- MET ============================================================================= Standard: Supply warnings in the documentation when the above goal has not been met. ----------------------------------------------------------------------------- NA ============================================================================= Goal: Arrays should be dynamically allocated. ----------------------------------------------------------------------------- NA ============================================================================= Standard: The characteristics of I/O should be clearly documented (i.e. the implementation of I/O unit numbers, if any). ----------------------------------------------------------------------------- MET (The information appears in a fortran comment line near the bottom of page 6 of the documentation.) ============================================================================= Goal: Avoid using hard-wired I/O unit numbers. Allow informational output to be switched on or off. Provide a method for rerouting warning or error message output to a user specified file or I/O unit number. ----------------------------------------------------------------------------- MET ============================================================================= ============================================================================= ** DOCUMENTATION STANDARDS ** ============================================================================= Standard: Provide name of contact person for support. ----------------------------------------------------------------------------- MET ============================================================================= Standard: Provide date of last revision. ----------------------------------------------------------------------------- MET ============================================================================= Standard: Provide at least comments describing module or code, citations to publications (if any), and range of validity. ----------------------------------------------------------------------------- MET ============================================================================= Standard: Specify the precision of floating point calculations. ----------------------------------------------------------------------------- MET The code is written so that the level of precision is controlled with compiler options, but there are no explicit instructions or reference to precision apart from a comment in the revision history. Perhaps this should be explicitly stated in the documentation. ============================================================================= Goal: offer single and double precision versions or offer user control of precision at compile time. ----------------------------------------------------------------------------- MET ============================================================================= Standard: Provide the index of input-output variables for each module (include type of variabgle, dimension, units). ----------------------------------------------------------------------------- MET ============================================================================= Standard: Provide statement of known bugs. ----------------------------------------------------------------------------- I could not find any statement about bugs. ============================================================================= Goal: Index of modules, routines, variables. ----------------------------------------------------------------------------- NA ============================================================================= Goal: Publication of code or module in journal (such as Computer Physics Communications). ----------------------------------------------------------------------------- NOT MET ============================================================================= Goal: Online hyper-text reference documentation. ----------------------------------------------------------------------------- NOT MET ============================================================================= Goal: Interactive online help menus. ----------------------------------------------------------------------------- NOT MET ============================================================================= ============================================================================= *============================================================================= **DATA STANDARDS** ============================================================================= ============================================================================= Goal: Provide interface routines to data. ----------------------------------------------------------------------------- NA ============================================================================= Goal: Use self-describing data files (such as NetCDF). ----------------------------------------------------------------------------- NA ============================================================================= Goal: Use public domain, portable, available and well-documented data file formats. ----------------------------------------------------------------------------- NA ============================================================================= Goal: Establish standards for variable names, units, dimensions independent variables and grid descriptions as they appear in the module interfaces. ----------------------------------------------------------------------------- NA =============================================================================