Re: [boost] Numeric Conversion Formal Review

From: Kevin Lynch (krlynch_at_[hidden])
Date: 2003-12-19 17:28:32

Thorsten Ottosen wrote:
> Dear all,
> 
> The review of Fernando Cacciola Numeric Conversion library starts today (8th
> of Dec.) and runs until the
> 22th of December.
> 

I regret to report that I have not been able to make the time for a 
complete and proper evaluation of the nitty gritty details of the 
current design and implementation, and I won't be able to before the end 
of the the review period, but I was deeply involved with the discussions 
about a year ago that let Fernando to build this library.  Hopefully 
makes up for the shortcomings in my evaluation this time around :-) 
Apologies if I bring up something that has already been discussed and 
addressed.... pretend I didn't mention it here.

>   a.. What is your evaluation of the design?
> 

I like the overall design.  I think that the design is well seperated 
into orthogonal components, is complete, and doesn't attempt to promise 
any more than it delivers.

>   b.. What is your evaluation of the implementation?

I regret that I have not evaluated the current implementation beyond a 
quick looksee.

> 
>   c.. What is your evaluation of the documentation?
> 

As noted by others, the documentation could use some help ... but what 
programmer's documentation doesn't need some help?  :-)

Specifically, as others have mentioned, there is a need for more 
examples to help people implementing their own traits and converters.

Additionally, I have the following comments and/or questions:

1) The Overview really needs to have a few motivating use cases, right 
up front, on the problem the library attempts to solve.  This is 
different from the need for examples of how to implement the traits 
needed to work with UDTs

2) In the definitions section:

a) Nitpick: I think that your definition of "floating numeric types" is 
too narrow; it leaves a gap in the numeric types that isn't filled by 
the other types.  By defining floating types to be "[where] the abstract 
values represent real numbers", you are only admitting numeric types 
such as float/double etc with vanishing density.  You leave out the 
class of numeric types with fraction parts and finite density (for 
example, a type that covers monetary values with a finite number of 
decimal digits are not included in your classification scheme).  I think 
you intended to include those kind of types in floats, because in at 
least two places, you mention that "floating types have a density << 1". 
  I think you need to change the definition.

b) Another nitpick: You define next() and prev() in the section on range 
and precision, and use that to define a numeric set by iteration.  Fine. 
  In the section on rounding, you use next() and prev() to define a 
"correct rounding" of an abstract value V to its rep v = rep(V) to be 
those roundings where v == prev(V) or v == next(V).  This is incorrect 
when the abstract value can be exactly represented: if V = 1 and T = 
int, then next(V) = 2, not 1.   I think you either need to redefine 
correct rounding in terms by adding the exactly representable case 
explicitly, or requiring "v == prev(next(V)) or v == next(prev(V))" 
which solves the problem.  The former is clearer, the latter is more 
succinct and prettier to my eyes  :-)

c) In the section on Standard Conversions, you use "sequel" a few times. 
  I can't figure out what you intended to say.  Did you mean "sequence" 
perhaps?

d) Comment: It seems strange that the standard demands that integers 
always be convertable to floats ... with this requirement, it is not 
possible for an implementation with the minimal compliant floating point 
(FLOAT_MAX = 1E+37) and big integers (128 bit longs, for instance, with 
ULONG_MAX approx 3.4E+38) to be compliant.  Granted, that would be a 
pretty strange platform, but it seems a bit wacky to preclude those 
implementations from conformance.  Anyone know of a good reason why the 
C++ Standard demands that integer to floating point _always_ be defined, 
while C99 explicitly does not (C99 6.3.1.4/2 has nearly identical 
language to C98 4.9, but additionally has the line: "If the value being 
converted is outside the range of values that can be represented, the 
behavior is undefined.")????   I don't have a copy of C89, so I don't 
know if that is a change to C or an incompatibility added by C++.  It 
isn't a "Defect" in the standard per se, but it is a little strange.

3) Header ... converter_policies.hpp

You give explicit values in the documentation to the values of enum 
range_check_result, which I don't think you should do in the 
documentation ...

>   d.. What is your evaluation of the potential usefulness of the library?

This library should not only be extremely useful, it plugs a yawning gap 
in the C++ standard library.  It should be useful in many numerically 
oriented libraries and applications where the screwy C++ conversion 
rules can really bite you.

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

I have not tried to compile or use the current implementation, but I did 
use the older implementation which Fernando made available a while ago; 
I had compiler related issues (Redhat gcc 2.96) at the time to work 
around that probably aren't relevant anymore.

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

This time around, I was only able to put in about two hours into 
studying the documentation and examples, so probably more than a quick 
read, but less than an in-depth study.

> 
>   g.. Are you knowledgeable about the problem domain?

Moderately ... I use numerics regularly in my job, but I have no formal 
training in the field.  I get bitten by the problems this library solves 
regularly enough to know the need.

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

I vote unconditionally for acceptance.

-- 
-------------------------------------------------------------------------------
Kevin Lynch				voice:	[phone]
Physics Department			Fax: [phone]
Boston University			office:	 PRB-361
590 Commonwealth Ave.			e-mail:	 krlynch_at_[hidden]
Boston, MA 02215 USA			http://budoe.bu.edu/~krlynch
-------------------------------------------------------------------------------