Hi all,
The review of the proposed Boost.Multiprecision library authored by
John Maddock and Christopher Kormanyos has been scheduled for
June 8th - June 17th, 2012
and will be managed by myself.
I hope everyone interested can reserve some time to read through the
documentation, try the code out, and post a formal review, either during
Le 29/05/12 23:08, Jeffrey Lee Hellrung, Jr. a écrit :
the formal review window or before.
Hi,
glad to see that the library will be reviewed soon.
I have spent some hours reading the documentation. Here are some
comments and a lot of questions.
* As all the classes are at the multi-precision namespace, why name the
main class mp_number and not just number?
typedef mp::number > my_float;
* I think that the fact that operands of different backends can not be
mixed on the same operation limits some interesting operations:
I would expect the result of unary operator-() always signed? Is this
operation defined for signed backends?
I would expect the result of binary operator-() always signed? Is this
operation defined for signed backends? what is the behavior of
mp_uint128_t(0) - mp_uint128_t(1)?
It would be great if the tutorial could show that it is possible however
to add a mp_uint128_t and a mp_int256_t, or isn't it possible?
I guess this is possible, but a conversion is needed before adding the
operands. I don't know if this behavior is not hiding some possible
optimizations.
I think it should be possible to mix backends without too much
complexity and that the library could provide the mechanism so that the
backend developer could tell to the library about how to perform the
operation and what should be the result.
* Anyway, if the library authors don't want to open to this feature, the
limitation should be stated more clearly, e.g in the reference documentation
"The arguments to these functions must contain at least one of the
following:
An mp_number.
An expression template type derived from mp_number.
"
there is nothing that let think mixing backend is not provided.
* What about replacing the second bool template parameter by an enum
class expression_template {disabled, enabled}; which will be more
explicit. That is
typedef mp::mp_number, false> my_float;
versus
typedef mp::mp_number,
mp::expression_template::disabled> my_float;
* As I posted in this ML already I think that allocators and precision
are orthogonal concepts and the library should allow to associate one
for fixed precision. What about adding a 3rd parameter to state if it
is fixed or arbitrary precision?
* Why cpp_dec_float doesn't have a template parameter to give the
integral digits? or as the C++ standard proposal from Lawrence Crow
(http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3352.html),
take the range and resolution as template parameters?
* What about adding Throws specification on the mp_number and backend
requirements operations documentation?
* Can the user define a backend for fixed int types that needs to manage
with overflow?
* Why bit_set is a free function?
* I don't see nothing about overflow for cpp_dec_float backend
operations. I guess it is up to the user to avoid overflow as for
integers. what would be the result on overflow? Could this be added to
the documentation?
* can we convert from a cpp_dec_float_100 to a cpp_dec_float_50? if yes,
which rounding policy is applied? Do you plan to let the user configure
the rounding policy?
BTW, I see in the reference "Type mp_number is default constructible,
and both copy constructible and assignable from: ... Any type that the
Backend is constructible or assignable from. "
I would expect to have this information in some way on the tutorial. I
will appreciate also if section "Constructing and Interconverting
Between Number Types" says something about convert_to<T> member function.
If not, what about a mp_number_cast function taking as parameter a
rounding policy?
* Does the cpp_dec_float back end satisfies any of the Optional
Requirements? The same question for the other backends?
* Is there a difference between implicit and explicit construction?
* On c++11 compilers providing explicit conversion, couldn't the
convert_to function be replaced by a explicit conversion operator?
* Are implicit conversion possible?
* Do you plan to add constexpr and noexcept to the interface? After
thinking a little bit I'm wondering if this is this possible when using
3pp libraries backends that don't provide them?
* Why do you allow the argument of left and right shift operations to be
signed and throw an exception when negative? Why don't just forbid it
for signed types?
* Why the "Non-member standard library function support" can be used
only with floating-point Backend types? Why not with fixed-point types?
* What is the type of boost::multiprecision::number_category<B>::type
for all the provided backends? Could the specialization for
boost::multiprecision::number_category<B>::type be added in the
documentation of each backend? and why not add also B::signed_types,
B::unsigned_types, B::float_types, B::exponent_type?
* Why have you chosen the following requirements for the backend?
- negate instead of operator-()
- eval_op instead of operator op=()
- eval_convert_to instead of explicit operator T()
- eval_floor instead of floor
Optimization? Is this optimization valid for short types (e.g. up to 4/8
bytes)?
* As the developer needs to define a class with some constraints to be a
model of backend, which are the advantages of requiring free functions
instead of member functions?
* Couldn't these be optional if the backend defines the usual operations?
* Or could the library provide a trivial backend adaptor that requires
the backend just to provide the usual operations instead of the eval_xxx?
* How the performances of mp_number
will compare with float?
* And last, I don't see anything related to rvalue references and move
semantics. Have you analyzed if its use could improve the performances
of the library?
Good luck for the review. A really good work.
Vicente
