Re: [boost] [Review][PQS] Physical Quantities System review (May 31 through June 9)

From: Matthias Troyer (troyer_at_[hidden])
Date: 2006-06-09 12:26:44

Hi, unfortunately I did not find as much time as I wanted to, due to  
other commitments but I will still give my impressions so far.

>  - What is your evaluation of the design?

While potentially a very useful library, there are some problems with  
the current design that make it, as already mentioned by a few  
others, unsuitable for serious physical computations. Below I will  
mention that issues that I found in a quick reading:

* The name t1_quantity is not well chosen, but that was already  
discussed before.

* The conversion factor between units only allow for 6 digits  
accuracy. This is a serious problem when using non-SI units, e.g.  
when measuring the energy in eV. Any conversion can incur an error of  
about 1e-6, which is larger than the accuracy of some simulations.  
The accuracy has to be increased. An arbitrary precision rational  
number would be needed.

* When using float and units common in quantum simulations, where  
hbar is set to 1, implying a conversion factor of about 1e-34, one  
gets a serious underflow when looking at common terms like hbar^3 or  
hbar^4. Hence there is a need for supporting other unit systems than  
SI as the base.

* The problem of BOOST_PQS_INT32 instead of using int32_t was already  
mentioned, and I will not go into details.

* Similarly, dimensioned quantities should behave just as plain  
floating point types for all arithmetic operations - at least when no  
unit conversions take place. This, and the problem of  
BOOST_PQS_COMPARISON_EPSILON was already mentioned by Peter  
Henderson. I will thus again not repeat the details.

* Finally the extension mechanism to define new units is awkward and  
will soon lead to serious conflicts. My two main concerns are:
  - can't you provide a simple macro implementing the extension?
  - is the Abstract_quantity_id  really necessary? Can't it just be  
replaced by the scale factor except for those cases where the scale  
factor for two units is the same and one needs to distinguish them.  
The immediate problem with the current design that I can see is that  
you recommend programmers to use the first available id for their new  
unit. If now two programmers define a new unit, these will have the  
same abstract id, and cannot be used together without changing the  
code. Using the scale factor as part of the abstract id would make  
such clashes much less likely.

>  - What is your evaluation of the implementation?

I have not studied the implementation.

>  - What is your evaluation of the documentation?

While not being optimal, one can learn what is needed. The one  
section that completely confused me and needs serious improvements is  
the concept documentation and the "Defintiion of Terms", hat Dave  
Abrahams already wrote about in detail - I will thus not repeat my  
concerns here but support his view.

>  - What is your evaluation of the potential usefulness of the library?

An improved library, solving the issues above, would be very useful.  
In the current form it is usable only for toy problems, and student  
homework assignments, but not for serious calculations.

>  - Did you try to use the library? With what compiler? Did you have
> any problems?

No.

>  - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?

About 4 hours, a quick reading.

>  - Are you knowledgeable about the problem domain?

Having worked for 16 years in the domain of high performance  
simulations of physical systems, I believe I can answer this with yes.

>  - Do you think the library should be accepted as a Boost library?

Not in the current version which is not suitable for high-accuracy  
scientific computations.