Hi Noel, "Noel Belcourt" <kbelco@sandia.gov> wrote in message news:0b4618cc0e11d126ba3201b67da5eaae@sandia.gov...
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.
Which bits are superfluous?
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.
Yes.
- What is your evaluation of the implementation? Too big. I believe this problem can be solved with significantly less code.
Ok. Your challenge is accepted.... :-)
- What is your evaluation of the documentation? Good although I didn't read it all.
Ok. Thanks !
- 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.
FWIW I am reasonably proud of compile time performance, but would be very happy if it can be improved.
- 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?
A typical use case for t2_quantity might be as a variable in a drop down list. The user selects a particular unit. A t2_quantity can record their selection. Typically only a few units are required. FWIW there is an implementation of t2_quantity in a previous version of pqs: http://www.servocomm.freeserve.co.uk/Cpp/pqs-2-00-02/pqs-2-00-02.zip The name rt_quantity is used there rather than t2_quantity. Examples of use are : <pqs_2-00-02/examples/bladecalc_example.cpp> (See the scratch struct in bladecalc_example.hpp too) <pqs_2-00-02/examples/mapped_rt_quantity.cpp> <pqs_2-00-02/rt_units_map.cpp> <pqs_2-00-02/capacitor_time_curve.cpp> A typical possible use case for the t3_quantity is to read an XML database and construct a quantity from the data there. OTOH that hasnt been implemented so far. Relevant info might be http://unitsml.nist.gov/ They are both cases of increasing amounts of the data that is arriving at run time rather than compile time.
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.
Ultimately I could probably provide a utility so that you could generate headers from either command line, or text files etc. The other point is that the selection of predefined quantities should be much bigger. See Appendix B.8 and B.9 of the Guide for the International system of Units. Also there would ideally be a central online database that could be queried, and user could generatedownload headers they want (which might also be generated and added form their data. How practical that is I have no idea though.
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?
The t2_quantity uses t1_quantity internally. Having not implemented the t3_quantity, I am not too sure what its form would be yet.
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.
Sure. I generated these files from a local database as my TMP skills are limited. Again I want to stress that ideally the socalled named-quantities the user wants should be provided . Currently the small selection is just for demo purposes. There could be potentially be a huge number of quantities included but usually the user only needs a small selection, so that wont affect compilation time etc..
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.
Automatic conversions are heavily integrated into the library. (e.g every operation, initialisation and assignment etc). Is it possible to give some detail on where conversions are likely to cause problems?
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.
The named_quantity headers provided are provided for general purpose use. However where only certain units are acceptable my suggestion would be to create similar headers but which incorporated only the acceptable units and possibly using local naming conventions. On the general issue of efficiency, Boost.Typeof can make a big difference, because the BOOST_AUTO facility can be used to create variables of unknown type and units but which should represent the most efficient result of a calculation. When the programmer creates their own intermediate results this can cause unwanted scaling, because they trigger a unit conversion. I hope to cover this in the docs but had concerns about including a Typeof dependency when Typeof was only available in CVS.
7) t1_quantity copy constructors don't check for copy to self.
OK. Is this dangerous? t1_quantity doesnt use pointer members etc. Or is efficiency the issue? I dont understand?
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.
This is a case of compile time optimisation. All I can say is that integers were originally used but rationals were only incorporated by popular demand. Experimenting with trying to reduce rationals to integers when possible actually increased compile times overall. The current rationals used are quite efficient IMO e.g compared with the mpl versions for example.
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)
force * distance --> ? might be torque or energy.
10) Is there any code or data overhead from the i/o support if I don't use any i/o?
Not from PQS. I/O support is designed to be modular and only include iostreams when specifically i/o headers are included. However I cannot guarantee that iostream headers are not brought in via other boost headers over which I have no control. Thanks for your review. regards Andy Little