[boost] [Review][PQS] Review

From: John Phillips (phillips_at_[hidden])
Date: 2006-06-08 18:00:39

    Andy, Fred and all
    Here are my thoughts on the PQS (which I agree is not a good name).
Some of them have become redundant since I began jotting them down, but
I'll add my voice even though Andy already knows about the issues. I
wish I had time to be more complete, but this is all I have time to do
at the moment.
    I also notice that although I was looking at the PDF version of the
documentation, there is an html version, that I have not looked at
closely enough to have any opinion on whether it is better.

About the documentation

First an issue of ordering.
    I think the Getting Started section is quite useful, but it is buried
a bit too far into the documents. My guess is that a typical user will
want this before anything else, so it should be front and center. This
can repair itself when the documents are done as hyper-linked text for a
browser instead of a PDF file, since a navigation area can highlight the
easy access to this information.
    Also, the Definition of Terms section is a Glossary, and is probably
better placed at the end instead of at the beginning. An understanding
of these terms is useful when digging through the details of the
library, but not nearly as useful for the causal user. Placed before the
Getting Started section, it will make many users wonder if this is going
to be too much work for the return.

Next one of convenience
    The Predefined t1_quantities list is not yet very useful. It is also
likely to be difficult to maintain, as your list of available units gets
longer if it is made useful.
    What is it missing?
    A way to decide what is actually available and how to use it.
    By browsing the headers, I find that in some cases, you have a true
wealth of possibilities. Your measures of length include such things as
rods and Astronomical Units, for example. (It also includes some things
that I don't understand putting in that header (such as acres), but
possibly you have a reason for this.) However, the documentation does
not yet give me any way to know what the available possibilities are. It
also doesn't tell me what the scales are for the various non-SI units,
so I have no way to decide that some non-SI unit might better represent
the scale of my problem.
    Unfortunately, fixing this problem will make the documentation quite
unwieldy for you to maintain. Just a table listing the name, notation in
the pqs and scale for each of the length units would currently have 19
entry rows (counting one for meters, but not including acres or
acre-feet). As other units are included, this format is far too likely
to be painful for you. I don't yet have a good idea for an alternative,
but I think one needs to be found.

One about consistency
    In the Concepts section, you have a reasonable framework for
describing each concept. Unfortunately, there are formatting issues that
make the concept descriptions hard to follow. The concept names are not
differentiated from the subsections of the concept descriptions in a
visually obvious way. Some use of a different font size or indentation
will make browsing the list for a concept description much easier.
    The first couple of descriptions have a consistent and complete
style, that quickly disappears in the latter concept descriptions. I
think this is probably just because you ran out of time for writing
them, but I want to make sure it isn't missed latter.

About "Getting Started"
    The pqs includes an all_types.hpp header, that is a one stop include
for all of the available types. Unfortunately, I see no place in the
documentation where you mention this. Instead, you include long lists of
includes when you want to use multiple types. Most new users will
probably find this a barrier, since the first concern will be using the
library, and only later will the users turn to reducing compile times
and sizes (if needed). I think the first couple of examples should just
use the all_types.hpp header, and in a later example point out that the
user can choose to only include the segments of the library that are
needed. Maybe even create an "all_types.hpp" at a higher directory level
that includes the types, the I/O capabilities and the whole thing. This
is not a good solution for the tightest include lists possible, but it
is great for someone who doesn't want to have to figure out which pieces
are where for the first few uses of the library.
    The examples in the Getting Started section are fine for getting the
basic uses of the library across. It is clear that the additional typing
is minimal and that the code really does become more clear. This could
be improved somewhat by showing how to convert a couple of toy
calculations in the section. Start from what might have been written
originally, and show how it is more readable with minimal intrusion and
improved error checking after conversion. Many of the first time users
will be people who are struggling with unit problems in a partially
constructed code, so make the Getting Started section reflect likely
user experiences.

About the examples
    After the Getting Started section, the next place someone is likely
to go is the "Examples." I would like to see them accessible from the
documentation, with capsule explanations of what is important in each
one, so the new user can pick and choose what is important for the
current uses.
    The examples themselves are nice. They just need an easy entry for
the user.

About the implementation

    The implementation appears reasonable, though I have not been able to
put the time into it that would be required to understand all of the
details. My primary issue is with the attachment between specific units
and prefixes. For example, in the length units, the prefixes for powers
of ten are connected only to the meters units. In my work, attaching
them to other units (such as MeV for mega electron volts or Gpc for
Giga-parsecs) is quite common. Without developing a detailed
understanding of the implementation, I have no suggestion for how to
decouple the prefixes from the specific units, but it is a question
worth considering. The prefixes are in common use with a wide range of
base units, so some method of decoupling strikes me as a good choice.

    In use, the impact of the names for the quantities is not a problem
for me. There is increased typing in some places, but the added clarity
and the reduced need for additional descriptive comments in the code
convince me that it is worth it.

My use of the library

    I only had a couple of hours to give to this, so I did not get to do
anything more than read the documentation, skim the implementation and
try a few of the examples. I would like to rework a couple of pieces of
simulation code to use this, but I just can't give that time at the
moment. From my small experience of it, I think it is a good starting place.
    In compiling the examples, I have had a few issues. I'm waiting to
report them until I have the time to formulate a small example that
displays them, or even better to find the problem. I get compiler errors
in some cases, using OSX 10.3.9 and gcc 3.3.

My one request

    I think this will become a reality with the addition of the type_of
library, but I strongly request an easy way to add new units. In my work
writing physical simulations, most of the time I use simulation units
instead of any standard physical unit. This is done for the usual reason
of keeping the numbers in the calculation as close to order 1 as
possible, to minimize the numeric issues. Only rarely is there a
convenient physical unit for this, so simulation units and various
dimensionless quantities are the rule for me. As such, I will often have
the need to define new units when using this system, so I would like it
to be as simple as possible.

My understanding of the problem domain

    I have a reasonable understanding of the issues encountered while
writing physical simulations, and only a passing understanding of the
implementation issues involved in writing a library of this sort.

Recommendation

    I am in favor of including this library in Boost. As noted above,
there are issues I would like to see addressed, but on the whole I think
this is a good piece of work and fills a very large need in the
computational science and engineering community.

                        John Phillips