[boost] [Review] Quantitative units review results
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 quantitySI::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 _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/ listinfo.cgi/boost
participants (1)
-
John Phillips