
Boost : 
Subject: Re: [boost] [review] Multiprecision review (June 8th  17th, 2012)
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 20120616 06:19:43
Hi, here it is my review which is a summary of the discussion we have
the past week.
The scope of the library must be stated more clearly, I don't think the
library is able to manage with fixedpoint arithmetic (as least as I
understand it).
The reasons why mixed arithmetic is not provided must be added in the scope.
The disadvantages to use mp_number instead of the underlying backend
when expression templates are not used and the backend is lightweight.
> What is your evaluation of the design?
While I was expecting a more open design/interface, I think that the
current design is good enough for some application.
* Separating the expression template and the specific number
representations (frontend/backend) is a good design decision.
* Mixing backends
It is unfortunate that the frontend doesn't takes care of mixed
arithmetic yet.
I would expect the result of unary operator() always signed. This
operation is not defined in the front end as it isn't unary operator+().
I would expect the result of binary operator() always signed.
Adding mp_uint128_t and mp_int128_t and assign it to a mp_int256_t needs
two explicit conversions. This interface limits some possible
optimizations.
I think it should be possible to mix backends without too much
complexity at least when expression templates are not used. When ET are
used, the ET need to take as parameters the backend of the parameters
and the result.
If mixing independent backends (that is BE provided by 3pp) is not
desired, I think that mixing BE of the same "family" (that is provided
by the same library/developer) is a must on some domain. The backend
could contain this metadata in some way (a typedef).
* backend requirements
It should be clear what are the backend requirements, which are
mandatory, which ones are used only if a specific operation of the
frontend is used and which one are used only if present as an optimization.
Each backend should document the requirements it satisfies.
* I would expect the library provide a backend for representing Nbits
integers with exactly (N/8)+1 bytes or something like that.
* And also an overflow aware backend for fixed precision integers.
* Allocators and precision are orthogonal concepts and the library
should allow to associate one for fixed precision. Adding a 3rd
parameter to state if it is fixed or arbitrary precision should help in
this concern.
* Conversions
IIUC convert_to is used to emulate in c++98 compilers C++11 explicit
conversions. Could the explicit conversion operator be added on
compilers supporting it?
The frontend should make the differences 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?
* shift operations
An overload of the shift operations with a unsigned should be provided
(in addition of the signed one) in order to avoid the sign check.
* Uses of bool
The use of bool in template parameters could be improved by the use of
an enum class which will be more explicit. E.g
enum class expression_template {disabled, enabled};
enum class sign {unsigned, signed};
* Default for ExpressionTemplates parameter
The ExpresionTemplate parameter could be defaulted to e.g.
ExpresionTemplate = expression_template_trait<Backend>::value
Where expression_template_trait could be defined with a defaulted value as
template<typename Backend>
struct expression_template_trait {
static const expression_template value =
Backend::expression_template_value;
};
Another defaulted value could also be possible, as disabled(or enabled) by
default
template<typename Backend>
struct expression_template_trait {
static const expression_template value = expression_template::disabled;
};
But I think a default expressed by the backend is a better default.
* noexcep:
The library interface should use the noexcept (BOOST_NOEXCEPT, ...)
facilities.
* constexpr:
It is unfortunate that the generic mp_number front end can not make use
contexpr as not all the backends can ensure this.
It will be worth however seen how far we can go.
* literals
The library doesn't provide some kind of literals. I think that the
mp_number class should provide a way to create literals if the backend
is able to. For example
to_nonnegative<2884,4>()
will produce a nonnegativefixed point constant with a range and
resolution just sufficient to hold the value 2884*2^4.
Adding a move constructor from the backend maybe could help.
constexpr mp_number<BE>::mp_number(const BE&&);
> What is your evaluation of the implementation?
The performances of mp_number<a_trivial_adaptor<float>, false>respect to
float and mp_number<a_trivial_adaptor<int>, false> and int should be
given to show the cost of using the generic interface.
Boost.Move should be used to emulate move semantics in compilers don't
providing it.
> What is your evaluation of the documentation?
The documentation is quite good, but there are some issues that need to
be taken in account, in particular:
* The documentation should contain Throws specification on the mp_number
and backend requirements operations.
* The tutorial should add more examples concerning implicit or explicit
conversions.
* The reference should state clearly which kind of rounding is done
while converting between types.
* The reference should contain the type of
boost::multiprecision::number_category<B>::type for all the provided
backends,
and why not add also B::signed_types, B::unsigned_types, B::float_types,
B::exponent_type?
* The reference section must contain the relation between files and what
is provided by them.
* The documentation must explain how move semantics helps in this domain
and what the backend needs to do to profit from this optimization.
* The rounding applied when converting must be documented.
* deviations from builtin behavior: The library say that the behavior
follows as far as possible the behavior of builtin type. The
documentation should state clearly (maybe in a specific section) the
specific differences.
> What is your evaluation of the potential usefulness of the library?
This library provides a uniform interface for multiprecission number 3pp
libraries adding some optimization using expression templates, which is
in its own quite useful.
> Did you try to use the library? With what compiler? Did you have
> any problems?
I tried it with some compilers and found just some errors with
clang2.9. Note that the errors are not present with clang3.0.
> How much effort did you put into your evaluation? A glance? A
> quick reading? Indepth study?
An in depth study of the interface to report what is missing to use it
for my fixedpoint library. I have not really take a look at the
implementation.
> Are you knowledgeable about the problem domain?
I don't know too much about floating points. For the time been, I'm only
interested in fixed point types.
>
> And, most importantly, please explicitly answer the following question:
>
> Do you think the library should be accepted as a Boost library?
Even if the current state of the library don't allows me to use it for
my fixed point library I guess that the authors will add the needed
features before I could provide similar features myself.
Anyway, the library provides enough features that are missing in Boost,
so YES, this library should be accepted.
 afficher le texte cité 
By order of priority:
First all the documentation issues:
* Document the size requirements of fixed precision ints.
Yes. This should be in the documentation before release. A fixed
precision integer backend reducing the size to the maximum will be welcome.
* Be a bit clearer on the effects of signmagnitude representation of
cpp_int  min == max etc.
Yes.
* Document cpp_dec_float precision, rounding, and exponent size.
Yes. this will be of course welcome.
* Can we be clearer in the docs that mixed arithmetic doesn't work?
I will add this even on the scope. And I think this is a must feature
for future versions of the library.
* Document round functions behaviour better (they behave as in C99).
Yes.
* Document limits on size of cpp_dec_float.
Yes.
Next some not too difficult features:
* Can we differentiate between explicit and implicit conversions in the
mp_number constructor (may need some new type traits)?
This is a must have, as the backend could have different expectations.
* Make fixed precision orthogonal to Allocator type in cpp_int. Possible
solution  add an additional MaxBits template argument that defaults to
0 (meaning keep going till no more space/memory).
Yes, this should be taken in account.
* Can the nonexpression template operators be further optimised with
rvalue reference support?
I think this must be analyzed.
* Can any functions  in particular value initialization  be made
constexp?
Unfortunately I think that the generic mp_number class can not take care
of this completely, but it should where possible.
I don't know enough the domain to have an opinion on the following:
* Add support for fused multiply add (and subtract). GMP mpz_t could use
this.
* Document std lib function accuracy.
* Make the exponent type for cpp_dec_float a templare parameter, maybe
include support for biginteger exponents. Open question  what should
be the default  int32_t or int64_t?
* Can ring types (exact floating point types) be supported? The answer
should be yes, but someone needs to write it.
* Should there be a choice of rounding mode (probably MPFR specific)?
Good luck,
Vicente
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk