General Comments

Documentation

  • The way the documentation prepared by doxygen is presented is still the same. It is not acceptable for a user documentation. It looks much more like a developer's documentation. This time Seth provided a general entry point for the Science Tools which gave access to the list of the available tasks. But this should be done automatically. In Jim Chiang's documentation (the best one by far), the 'real' doc for the user is found under the 'Related pages' item, really not the place where one would look first. I insist that the point of view should be entirely inverted. The user's documentation should come first, and the developer's doc second. In particular, the user does not care about the C++ classes.

General settings

  • Parameter interface
    The parameter interface does not handle gracefully wrong input. For example, if I make a typo in one parameter name, I get the error message 'Caught N5hoops12PILExceptionE at the top level: Could not open parameter file for exposure_map (at ../src/hoops_pil.cxx: 371)'. This is not very useful. It should write 'parameter soandso does not exist' so that the user has a chance to understand his mistake.
  • Verbosity
    I did not understand how I could choose the verbosity of the tasks. Does this have anything to do with the 'chatter' parameter? This doesn't seem to have any effect in either gtselect of gtbin.
  • Graphical User interface
    In principle, this is a nice development. However the current version suffers from serious flaws. It asks confirmation of all mandatory parameters. This is really a nuisance. When you use a GUI you see the parameter values on the screen, there is no need to ask confirmation again. It asks even if the mode is set to h. Probably for the same reason (because it asks questions), the GUI cannot be detached from the calling window. This is again not what you would expect from a GUI.
  • Parameters
    In general, the number of mandatory parameters is too large. It should be kept to a strict minimum, and the first time user should be told to use the gui to learn how to use the parameters and their default values.
    Also I suggest to edit guidelines so that a parameter which represents the same thing in different tasks (like the input event file) always has the same name. General guidelines on parameter names would help as well. For example, a parameter for an input file should start by 'in', a file name should end in 'file' (like inevfile).

gtbin

  • The description still refers to evtbin.
  • Graphical User interface
    The number of parameters is too large and they can't all be seen. They should be organized in folders depending on the value of the 'algorithm' parameter.
  • Parameters
    The only mandatory parameters should be the evfile and algorithm parameters. In theory, scfile (another input file name) should be too, but I could not understand why scfile is required so I am not sure.
  • Functionality
    There is a bug when building a map (algorithm=CMAP) in Galactic coordinates (coordsys=GAL). In that case the CTYPE1 and CTYPE2 keywords should start with GLON and GLAT respectively (not RA and DEC).

gtselect

  • The description of what the task does is very skimpy. It should contain at least a description of the context (to what aim do you use this task) and how it articulates with the rest of the software.
  • Parameters
    The only mandatory parameter should be the input_file parameter.
  • Selection functionality
    This is rather limited compared to what cfitsio allows. I don't believe it is worth developing our own selection syntax, so we should consider seriously the possibility of directly using cfitsio's syntax and routines for that purpose.

map_tools

  • I am not sure why this package still exists. As far as I know, count_map is entirely superseded by gtbin, exposure_cube by gtlivetimecube. In any case, this package should be harmonized with the rest of the Science Tools.
  • I could not run map_stats. It answered that it could not find a parameter file. Since it is not described what this task does, it is not clear it does better than the standard fimgstat.
  • The only task which does not appear to be deprecated is exposure_map. gtexpmap does something else (preparing work for gtlikelihood). I am not happy with those names. gtexpmap should be the task which builds a standard exposure map (like exposure_map does), not an obscure preparation step for gtlikelihood whose output is essentially useless except to gtlikelihood.
  • No labels