From: Noel Belcourt (kbelco_at_[hidden])
Date: 2006-06-08 02:54:51
Please always explicitly state in your review, whether you think the
library should be accepted into Boost.
Boost clearly needs a dimension/unit analysis library but I believe
there is a much smaller, leaner library hiding inside PQS. I recommend
we do not accept PQS at this time and explore smaller, lighter weight
solutions including a possibly revised PQS.
You might want to comment on the following questions:
- What is your evaluation of the design?
Seems like a lot of effort went into getting input and output of
quantities correct.
- What is your evaluation of the implementation?
Too big. I believe this problem can be solved with significantly less
code.
- What is your evaluation of the documentation?
Good although I didn't read it all.
- What is your evaluation of the potential usefulness of the library?
The scientific community has a real need for a library to enforce
consistent unit handling.
- Did you try to use the library?
Yes, I got a couple of "ambiguous use of partial specialization" and
"illegal explicit template specialization" errors with CW 10 on Darwin.
I'd still like to do some side by side comparisons of compile times
with another PQS-like library but haven't had time yet.
- How much effort did you put into your evaluation?
A few hours assessing capability and possible integration into our
finite element codes.
- Are you knowledgeable about the problem domain?
I am knowledgeable about the domain.
I have some questions and general observations for Andy.
1) Can you give examples of the typical use cases for the proposed t2
and t3 quantities?
2) I'm concerned that the derived dimensions have to implement three
separate files, one for t1_quantity/types, t1_quantity/types/out and
pqs/meta/components. This is not a scalable solution for people who
need to add or customize a number of derived dimensions for their
problem domain. Adding new types involves considerable work, even
though Andy's provided template files (xxxx.hpp and of_xxxx.hpp) to
help with the task.
3) Will the derived dimension code work with the forthcoming t2 and t3
quantity or will there be another potential combinatorial amount of
code to support these quantities later on?
4) Instead of making the user implement the substantially similar
derived dimensions files, maybe using template meta programming to
generate the code automatically would work better and might result in a
smaller, easier to maintain code base.
5) Is there any way to disable automatic unit conversion? Automatic
conversion is a major liability for the scientific community in
critical sections of computationally intensive codes and bugs related
to this could be very difficult to track down.
6) I appreciate arguments favoring readable code but, in this case, I
find that all the unit names (cm_div_s2, km_div_s2, ...) are error
prone and likely to reduce code reuse. I'd prefer a solution where I
define a unit system, such as the SI system, and then have all
quantities in my library use that unit system. This solution makes
verifying our algorithms work correctly with more than one unit system
very easy, no code changes, and makes it easier to write generic
algorithms that are unit independent. The explicit type solution in
PQS is, in my opinion, significantly more error prone. Consider this
scenario:
In a critical section of code, someone could cause repeated conversions
between quantities due to a typing error. A quantity declared cm_div_s2
is, 100 lines down the function, assigned it to a quantity declared
km_div_s2. This automatic conversion is tough to catch visually, won't
cause a compile error but will cause significant performance and
numerical problems.
7) t1_quantity copy constructors don't check for copy to self.
8) Is there an easy way to replace the rational dimensions with
integers? A number of disciplines could probably make do with integers
and paying for rational dimensions seems expensive and unnecessary,
although it's certainly more general.
9) Could you give an example of where an anonymous quantity is required
because PQS doesn't know which particular named quantity the programmer
intends the result to be? (from the definition of terms for
anonymous_quantity)
10) Is there any code or data overhead from the i/o support if I don't
use any i/o?
-- Noel Belcourt