[boost] [Review] Quantitative units review results

From: John Phillips (phillips_at_[hidden])
Date: 2007-04-11 09:37:20

I’m happy to report that the quantitative units library developed by
Matthias Schabel and Steven Watanabe has been accepted for inclusion in
boost. My thanks to the authors for their submission, and to the
reviewers for all the work put into the reviews and discussions. A very
busy review period saw contributions ranging from small commentaries to
extensive reviews and discussions from Shaun Bloom, Martin Bonner, Paul
Bristow, Malte Clasen, Phil Endecott, Michael Fawcett, Ben FrantzDale,
Dave Hicherson, Lewis Hyatt, Janek Kozicki, Zach Laine, Eric Lemings,
Kevin Lynch, John Maddock, Michael Marcin, Scott McMurray, Noah Roberts,
Martin Schulz, Andrey Semashev, Dave Steffen, Matthias Troyer, K. R.
Walker, David Walthall, Gunter Winkler and Deane Yang. Thanks to you
again, since without your work the library would not become nearly as
good as it can be.

Given the scale of the review and the large number of issues
discussed, this summary will be rather long. As has been my policy in
the past, I will try to encapsulate the main ideas in the discussions
for ease of later reference. In the case of this library, there are also 
some specific recommendations that should be addressed before the 
library is added to the CVS. Some of them were fixed by the authors 
during the review, and made available at that time, but for completeness 
the completed issues will also be included in the report.

I’ll organize the report with some general discussion issues first,
followed by suggestions for improvement for the library (required issues
will be clearly marked and listed first), and since there was a vocal
minority who argued against inclusion, the report will end by 
specifically addressing some of the points they raised in the review 
discussion.

DISCUSSION

Whether boost should include unit analysis as a compile time library or 
a runtime library is an almost religious issue. On each side are 
adherents who are strongly committed to their views and who see no way 
their needs can be met by the other type of library. However, the goal 
of boost is not to decide who is correct in situations of this sort. 
Boost was founded to generate good ideas and good implementations so use 
by a wide audience could determine which if any of these ideas are 
appropriate for addition to the language. Therefore, my suggested 
resolution to the compile time vs. runtime issue is for a runtime 
adherent to submit a runtime units library that is of sufficient quality 
for boost inclusion and let the broader base of programmers decide if 
there is a need for one or both libraries.

Since the compile time library is already completed and accepted, it is
reasonable to expect that any runtime submission should either be able 
to cleanly interoperate with the compile time version or a convincing 
rationale will be supplied for why this is not a reasonable goal. I look 
forward to this submission, though I will probably not volunteer to run 
the review since I claim no expertise in the issues that runtime 
proponents feel make a compile time solution inadequate.

For the moment, I believe there is certainly enough of an audience that 
is looking for a compile time solution to justify the need for a 
library. I could only find two reviewers who said that a proper solution 
couldn’t be formulated with compile time facilities. The implication is 
that the rest of the people who expressed opinions about the library did 
not consider this an obstacle. In a list of library builders (as the 
boost developer list is) I would expect a bias toward compile time 
solutions compared to runtime solutions, but not enough of a bias to 
make me believe there is not an audience for a compile time solution.

I also think that some of the problems perceived by those who opposed 
the library are more communications problems than problems with library
functionality. To try to alleviate this issue, I am insisting that the
documentation adds a rationale section. This section is suggested for 
all boost libraries, but is badly needed in a setting where there are 
many subtle mistakes of thinking or design that can be made. The authors 
showed during the discussions that they have good reasons for their 
choices, but some of those reasons are not obvious. A rationale section 
that is clear and complete on the topics of what the library is intended 
to do, why that is a good idea and how that can be done in C++ will go a 
long way to avoid further misunderstandings.

The library uses the type system to make code more readable and more
dependable. However, many commonly used third party libraries are not 
built in a way that is compatible with this use of the type system. 
Thus, some sort of proxy or façade system will be used in most cases to 
provide an interface with third party facilities. My personal preference 
is to put the special interface on the unsafe libraries, but either way, 
this would be a good choice for a simple example. It also gives a chance 
to explain that this is not a flaw in the design of the units library, 
but instead an issue with how to communicate between sections of a 
program that have incompatible type information. If, in the future the 
third party libraries are improved to allow direct communication, the 
façade can become a pass through or be removed.

Imperial units (My apologies to Martin for the use of the term.) open a
can of worms, but a nearly unavoidable one. They should either be 
included, or a prominent explanation of why they are not included should 
be provided.

During the review questions arose of what to do with units such as meter 
* furlong / foot. The decision that the default behavior is to not 
convert and simplify is IMHO the correct one. In my academic area of 
expertise, the units km / (s * Mpc) (kilometers per second per 
megaparsec) are an example of a commonly used unit where automatic 
simplification is not acceptable. Megaparsecs measure distance, but 
should not be used to simplify with the kilometers. (These units are 
used to measure the expansion rate of the universe.)

Paul made the case in his review, and I think he is correct, that the
documentation should be very friendly to non-expert C++ programmers. 
Many potential users will not be experts in the template system or even 
the type system, but will be looking for a way to write more usable code 
for manipulating numbers in calculations. The documents should be 
written and organized to aid this. My suggestions are that you simplify 
the quick start’s discussion of how the library does its job, and focus 
on just presenting two problems (the work calculation and the circuit 
calculation) and how the units library makes solving those problems 
easier and more intuitive. Do it as two problems, each with a reason to 
exist, instead of as one mashed together program. Also, the listings for 
each example on the examples page should include a short (a few words to 
a couple of sentences) description of the problem the example 
illustrates and solves. Remember that someone who doesn’t already know 
the value of including units in code (and that means almost everyone) 
will look at the introduction, the quick start and the examples. If they 
aren’t convinced of the utility of the library by then, they’ll stop 
looking.

This isn’t a binding suggestion, but right now static_rational is an
implementation detail. As such, I think it should be kept internal to 
the library. If it is useful and important enough to move out to its own
library, then pretty it up and submit it for the review process. This
closely follows the pattern established by such useful tools as fusion.

It took some thinking, but I believe the decision not to add 
operator++() is a good one. It might be good to include the reasoning 
and something like the example that ended that discussion (from Eric, I 
think) in the rationale section.

REQUIRED IMPROVEMENTS

Each of the improvements below should be addressed before adding the
library to the CVS.

Add a rationale section to the documentation.
Since the design decisions required for a good units library aren’t 
always obvious, you need to include a section that explains the choices 
and your reasons for making the design decisions you made. Many things 
listed below have a place in this section.

Fix the bugs in conversion/construction.
You already addressed this, but make sure each way to construct a 
quantity that should work, does work. Include an explanation of why the 
ones that shouldn’t work don’t.

Either include a real performance test, or no performance test.
As the discussion during the reviewed, performance testing is not 
trivial, and when done without careful thought creates confusion. If you 
can develop a good performance test, include it in the examples or as a 
separate program in the documentation. However, example 14 isn’t a good 
performance test and confuses people about what is happening in the library.

Check the suggestion about auto generation of ordinal values. If auto
generation works across translation units, use it instead of assigning
values. If auto generation does not work, consider the reserved range
ordinals. This has to be addressed before submission because any later
decision to change the range of reserved ordinals is a potentially 
breaking change for users (and probably a very hard one to debug). 
Remember that it is better to reserve 3 times too many ordinals than 
even 1 too few.

Look at the problems of reworking the library for mpl re-use and
simplifying the conversion implementation. If these projects are of a
reasonable scale to be completed without slowing you down so much that 
you miss a release, do them now. Otherwise, implement them after 
including the library in the CVS.

Remove the mutating member function x.value(). You have already agreed
that this is a good choice and that needed mutating functionality should 
use a more ugly and obvious syntax, such as x.quantity_cast().

You have designed a way to simplify reverse conversions so someone who
adds a new unit system doesn’t have to add redundant conversion 
definitions. Include this in the library and documentation.

Include a rationale item that explains why the default behavior is
explicit conversions.

Correct the typos Paul noticed on the work “calculation” in the quick
start.

Provide a clear and complete listing of the systems and units included 
in the library.

The use nautical::miles is not consistent with other names. Decide what
naming scheme you want and stick with it.

Include a rationale item that explains the reason for the notation
quantity<SI::length> m_value=3.0 * SI::meter. I think you have a good
reason, but it isn’t explained to the user.

SUGGESTED IMPORVEMENTS

The improvements below are not required, but they are suggestions for 
how to make a good submission better. Consider them, and do what you 
think will make the best library.

Consider a check for compilers that are known to not be able to compile
the library.

Carefully consider the purpose for each example provided.
What do you expect a reader to learn from each example? What examples 
are needed for users who want to write code for the common use cases for 
a library of this sort?

Add an item to the rationale to quickly explain why rounding and error
issues are not handled in the library.

Consider changing the notation for negative exponents away from using
parenthesis.

Zach Laine had a collection of suggested edits to the text. Consider how
to best fix them.

One thing that will greatly aid broad adoption is ease of definition for
new units and systems. There are tons of unit systems in the world, and
someone is using each of them. As such, the easier you can make adding 
new units and systems, the better. This includes a possible tutorial 
showing the process (including discussions of the decisions that must be 
made and good ways to make them). It will greatly benefit the authors if 
anyone who tries this tells them about what went well and poorly.

Provide clear and detailed examples of implicit and explicit 
conversions. Include a discussion of the good and bad points of each.

Consider convenience headers for a variety of typical use cases.

Consider whether you should change the current header arrangement to 
make them more or less fine grained.

Consider including a simple currency example, with rates checked at run
time from some simple source.

Consider an example of using the library to interface with a GUI 
(similar to Janek’s).

Consider including an example of how to accept a value in units not 
known at compile time and convert to a predetermined unit.

You decided the if() check on line 57 of quantity.hpp is not needed. 
Check for other unneeded code and remove.

Include the SI prefixes (milli, mega, …) in the SI catchall header.

Since you can’t assume the memory layout of quantities in arrays is the
same as doubles in all cases, consider an optional compile time check on
sizeof() to make sure the size is the same.

Consider adding the example from the discussion of a dual reference 
frame system. This is a good use of this library that won’t occur to 
many users without help.

Consider opening the constants for use in systems that don’t provide
native typeof(). This might be unimportant, if such systems can’t use 
the library for other compatibility reasons.

Documentation is the gift that keeps on giving. Continue to look for 
ways to make the documentation more clear and complete.

ISSUES RAISED WITH THE LIBRARY

During the discussion, several issues were raised that have reasonable
answers. I’ll try to collect my understanding of the issues and the 
answers here.

It does not allow interaction with a GUI layer.
Janek’s example posted to the discussion shows direct interaction
including the ability to select units at runtime from the GUI. Similar 
can be done for other presentation layer issues except for possible 
rounding issues, which are inherently hard to solve satisfactorily.

The focus should be on the business and presentation layers.
While both layers are important, there is a substantial audience for 
this library that does little to no work with them. If this library 
proves insufficient for this work, a runtime submission is encouraged.

Rational exponents are a needless complication and make integration with 
a runtime system impossible.
While not everyone has a use for them, there is a noteworthy population
that needs to have them, so they are provided. Similar functionality can 
be provided at runtime using boost::rational.

Inadequate serialization tools are provided.
I/O with units is a large and complicated problem all its own. Anyone 
who wishes to solve it is welcome to develop a library proposal, but the 
scope of this proposal is intentionally limited in this regard.

Workarounds are needed for third party libraries, such as BLAS.
This is true, but a common symptom of any unit system for C++. The only
question is then where the workarounds happen. See above for more on this.

Doesn’t support … (pick your favorite)
So far, Janek has provided code examples for GUI use and Michael tells 
us he has used it with OpenGL and other systems. In fact, reasonable 
solutions have been provided for every problem presented, so far. There 
may be problems of this sort, but no direct evidence has been seen so far.

No check for matching units on deserialized quantities.
This is true to an extent, but not as important as it sounds. The
serialization library provides support for version numbers on archives. 
The version number can be propagated between groups along with a 
definition of the required data format for that version. Yes, it is 
possible for someone to ignore this information, but it is also possible 
for them to call their units meters but use numbers that go along with 
feet. No unit system can eliminate such intentional circumventions, so 
it isn’t very important that this one doesn’t.

Some real world uses of units have improper dimensions. For example
measuring pressures in terms of inches of mercury.
This will probably best be handled at I/O time, and converted to some 
more sensible internal unit for the rest of the program. In cases where 
this is not a viable answer, I don’t currently have one.

Why distinguish between torque and energy, since the have the same
dimensions?
There are sensible physical problems where torque and energy both 
appear, such as rotational energy problems with work. As such, 
insulating against accidents is good.

That’s the whole thing.
Thanks to anyone who lasted this long, and thanks once again to the
authors.

John Phillips