[boost] [review] Multiprecision review (June 8th - 17th, 2012)

[Jeffrey Lee Hellrung, Jr.]

Okay, it's that time. Apologies for not sending a reminder, but there had
been a lot of pre-review comments over the last week or so, so I think this
has been on at least some people's radar. The original review announcement
is given below.

On Tue, May 29, 2012 at 2:08 PM, Jeffrey Lee Hellrung, Jr. <
jeffrey....@gmail.com> wrote:

> 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.
>
> From the Introduction:
>
> --------
> "The Multiprecision Library provides *User-defined* integer, rational and
> floating-point C++ types which try to emulate as closely as practicable the
> C++ built-in types, but provide for more range and precision. Depending
> upon the number type, precision may be arbitrarily large (limited only by
> available memory), fixed at compile time values, for example 50 decimal
> digits, or a variable controlled at run-time by member functions. The types
> are expression-template-enabled for better performance than naive
> user-defined types."
> --------
>
> And from the original formal review request from John:
>
> --------
> Features:
>
> * Expression template enabled front end.
> * Support for Integer, Rational and Floating Point types.
>
> Supported Integer backends:
>
> * GMP.
> * Libtommath.
> * cpp_int.
>
> cpp_int is an all C++ Boost licensed backend, supports both arbitrary
> precision types (with Allocator support), and signed and unsigned fixed
> precision types (with no memory allocation).
>
> There are also some integer specific functions - for Miller Rabin testing,
> bit fiddling, random numbers. Plus interoperability with Boost.Rational
> (though that loses the expression template frontend).
>
> Supported Rational Backends:
>
> * GMP
> * libtommath
> * cpp_int (as above)
>
> Supported Floating point backends:
>
> * GMP
> * MPFR
> * cpp_dec_float
>
> cpp_dec_float is an all C++ Boost licensed type, adapted from Christopher
> Kormanyos' e_float code (published in TOMS last year).
>
> All the floating point types, have full std lib support (cos sin exp, pow
> etc), as well as full interoperability with Boost.Math.
>
> There's nothing in principal to prevent extension to complex numbers and
> interval arithmetic types (plus any other number types I've forgotten!),
> but I've run out of energy for now ;-)
>
> Code is in the sandbox under /big_number/.
>
> Docs can be viewed online here:
> http://svn.boost.org/svn/boost/sandbox/big_number/libs/multiprecision/doc/html/index.html
> --------
>

Any review discussion should take place on the developers' list (
bo...@lists.boost.org), and anyone may submit a formal review, either
publicly to the entire list or privately to just myself.

As usual, please consider the following questions in your formal review:

What is your evaluation of the design?
What is your evaluation of the implementation?
What is your evaluation of the documentation?
What is your evaluation of the potential usefulness of the library?
Did you try to use the library? With what compiler? Did you have any
problems?
How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?
Are you knowledgeable about the problem domain?

And, most importantly, please explicitly answer the following question:

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

Lastly, please consider that John and Christopher have compiled a TODO list
[1] based on pre-review comments. Feel free to comment on the priority and
necessity of such TODO items, and whether any might be show-stoppers or
warrant conditional acceptance of the library.

Thanks in advance; looking forward to the discussion!

- Jeff (& John & Christopher)

[1]
http://svn.boost.org/svn/boost/sandbox/big_number/libs/multiprecision/doc/html/boost_multiprecision/map/todo.html

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Paul A. Bristow]

> -----Original Message-----
> From: boost-...@lists.boost.org [mailto:boost-...@lists.boost.org] On Behalf Of Jeffrey
Lee
> Hellrung, Jr.
> Sent: Friday, June 08, 2012 3:28 PM
> To: bo...@lists.boost.org; boost-a...@lists.boost.org; boost...@lists.boost.org
> Subject: [boost] [review] Multiprecision review (June 8th - 17th, 2012)
> > --------
> > "The Multiprecision Library provides *User-defined* integer, rational
> > and floating-point C++ types which try to emulate as closely as
> > practicable the
> > C++ built-in types, but provide for more range and precision.

> > C++ Depending

> > upon the number type, precision may be arbitrarily large (limited only
> > by available memory), fixed at compile time values, for example 50
> > decimal digits, or a variable controlled at run-time by member
> > functions. The types are expression-template-enabled for better
> > performance than naive user-defined types."
> > --------

> What is your evaluation of the design?

Boost.Multiprecision is an exciting development because it provides two items that Boost has long
needed in its toolkit:
fixed and arbitrary precision integer types,
and
fixed and arbitrary precision floating-point types.

That Boost.Math functions can be called directly is a massive step forward - even just for 'trivial'
tasks like pre-computing math constants.

Wheee! - we can now use brute force and effortlessly hurl heaps of bits at any recalcitrant
problems.

Suddenly, we can do all sorts of tricks at altogether monstrous precision and range! Getting more
precision for some of the calculation is suddenly painless.

(Of course limiting the hundreds-of-digits calculations to the nitty-bitty bits will reduce the
speed penalty).

(Not to mention random and rational, and that complex and fixed-point might also finally be made
fully generic is looking distinctly possible and that would be even more wonderful).

> What is your evaluation of the implementation?

License and backend
=================
Allowing a choice of backend has crucial license advantages, allowing the 'gold standard' optimised
GMP, but also allowing the Boost licensed version with remarkably little loss of speed. (I note the
price we are paying for the commercial greed and patent abuse that has made maintaining the GPL
license status of GMP such a quasi-religious issue).

Crucially, the implementation works hard to be as near as possible plug-in for C++ built-in types
including providing std::numeric_limits (where these - mostly- make sense). In general, all the
iostream functions do what you could (reasonably) expect. And it is a very strong plus-point that
fpclassify and all the usual suspects of regular C99 functions exist: this should mean that most
moves from built-in floating-point to multiprecision should 'just work'.

(Any potential license problems from copyright of Christopher Kormanyos's e_float by the ACM have
been resolved).

Speed
=====

Fast enough for many purposes, especially if it is possible to use a GPL backend. Optional
expression-template-enable is cool.

Testing
======

There is a big suite of test programs written by Christopher Kormanyos to test his e_float type
(which engine was hi-jacked by John Maddock to extend it to (optionally) use expression templates).
These provide a good assurance that the underlying integer and floating point types work correctly
and that it is going to work when used in anger.

Unsurprisingly, I was able to run the test package using MSVC VS 10 OK. Don't hold your breath!

(Testing iostream is, of course, a nightmare - there are an infinity of possible I/O combinations
and the standard is sketchy in places and there are some differences between major platforms, so
portability is never going to be 100%. But I got the impression that it works as expected).

Writing a simple loopback stream output and re-input, I found that using Boost.Test to compare
values can mislead.

A patch is at https://svn.boost.org/trac/boost/ticket/5758
#5758: Boost.Test Floating-point comparison diagnostic output does not support radix 10 (not enough
digits displayed)

For example, it leads to nonsensical reports from a loopback test like
[1e+2776234983093287513 != 1e+2776234983093287513]
when the true situation should be obvious from this
[1e+2776234983093287513 !=
9.99999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
9999999999999999999999e+2776234983093287512]

However the underlying Boost.Test macros appeared to work fine and are used by a comprehensive set
of tests provided (dealing with complications of multiple backend and error handling policies).

> What is your evaluation of the documentation?

I was able to write some trivia before I needed to dig in. Nice. Convincing user examples. Warns of
some dragons waiting to burn the unwary. (Very few typos - nice proof-reading ;-))

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

When you need it, you need it very badly. So essential.

> Did you try to use the library? With what compiler?

Used with MSVC 10 for a few experiments and to calculate high precision constants.

Did 'what it said on the tin', and agreed with Mathematica and other sources.

> Did you have any problems?

Shamefacedly, I fell into the pits noted below and was duly singed by the dragons lurking therein
:-(

> How much effort did you put into your evaluation?

Reasonable, including playing with e_float.

I wrote a simple loopback stream output and re-input using the random package.
ss << std::setprecision(std::numeric_limits<cpp_dec_float_100>::max_digits10) << b <<
std::endl;
It was essential to use max_digits10, not just digits10. This ran until I got bored.

Careful reading of docs.

Enough use to be confident it works OK.

> Are you knowledgeable about the problem domain?

Faintly.

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

Definitely YES.

> Lastly, please consider that John and Christopher have compiled a TODO list [1] based on
pre-review
> comments. Feel free to comment on the priority and necessity of such TODO items, and whether any
> might be show-stoppers or warrant conditional acceptance of the library.

No showstoppers.
==============
It is ready for use.

I am sure that the library will be refined in the light of wider user experience (and that will only
really come when it is issued as a Boost library).

Implicit conversion
===============
Initially e_float (like NTL - used during development of Boost.Math as an example of a
multiprecision type, and for calculation of constants) both forbade implicit conversion. But it
soon became clear that it was impractical to make everything explicit and we patched NTL to permit
this (and to provide the usual suspects of std::numeric_limits and functions too).

This leaves some dragons waiting for the unwary, so those who write

cpp_dec_float_100 v1234567890 =
1.23456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
012345678901234567890;

will get what they asked for, and deserve - the catastrophic loss of accuracy starting at the 17th
decimal digit!
(`std::numeric_limits<double>::max_digits10` = 17 for the common 64-bit representation).

This loss of accuracy will rarely jump out at you :-(

(If I get a £1, $1, or 1 euro for everyone who makes this mistake (or one of the many other complex
pits discussed in the docs), I believe I will become rich ;-)

It would be nice to catch these mistakes, but not at the price of losing use of all the Boost.Math
functionality (and much more).
(I fantasize about a macro that can switch intelligently between explicit and implicit to protect
the hapless user from his folly, but advising on loss of accuracy is probably really a compiler
task).

On the old hand, it is really, really cool that using a string works:
cpp_dec_float_50 df = "3.14159265358979323846264338327950288419716939937510";

Guard digits
==========
The existence and (surprising) number of these has already been discussed and I see no problem with
the way it works. It will be an exceptional program that really needs to use max_digits10 rather
than digits10. (Boost.Test is an example, to avoid nonsensical display of [2 != 2] when guard
digits differ - see above).

Paul

---
Paul A. Bristow,
Prizet Farmhouse, Kendal LA8 8AB UK
+44 1539 561830 07714330204
pbri...@hetp.u-net.com

[Marc Glisse]

On Sat, 9 Jun 2012, Paul A. Bristow wrote:

> License and backend
> =================
> Allowing a choice of backend has crucial license advantages, allowing
> the 'gold standard' optimised GMP, but also allowing the Boost licensed
> version with remarkably little loss of speed. (I note the price we are
> paying for the commercial greed and patent abuse that has made
> maintaining the GPL license status of GMP such a quasi-religious issue).

Let us try to be factual in a section called "License": the license of GMP
is *L*GPL. I know everything GNU is anathema in BSD-land, but it does make
a difference.
(end of digression, please no license flame war)

--
Marc Glisse

[Jeffrey Lee Hellrung, Jr.]

Paul: Thanks for taking the time to make such a detailed review!

[Jeffrey Lee Hellrung, Jr.]

I'd like to encourage those interested in John and Christopher's
Multiprecision library to take a close look at it at some point this week,
provide suggestions for improvement and/or discuss any potential issues,
and submit a formal review. Thanks!

Original announcement below.

On Fri, Jun 8, 2012 at 7:28 AM, Jeffrey Lee Hellrung, Jr. <

[Vicente J. Botet Escriba]

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 fixed-point 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 back-end
when expression templates are not used and the back-end 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 (front-end/back-end) is a good design decision.

* Mixing back-ends
It is unfortunate that the front-end 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 back-ends without too much
complexity at least when expression templates are not used. When ET are
used, the ET need to take as parameters the back-end 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 meta-data in some way (a typedef).

* back-end requirements

It should be clear what are the back-end requirements, which are
mandatory, which ones are used only if a specific operation of the
front-end is used and which one are used only if present as an optimization.

Each back-end should document the requirements it satisfies.

* I would expect the library provide a back-end for representing N-bits
integers with exactly (N/8)+1 bytes or something like that.

* And also an overflow aware back-end 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 front-end 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
clang-2.9. Note that the errors are not present with clang-3.0.

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

An in depth study of the interface to report what is missing to use it
for my fixed-point 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 sign-magnitude 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 non-expression 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 big-integer 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

[Jeffrey Lee Hellrung, Jr.]

The review of Multiprecision is due to end on Sunday, June 17th, 2012, but
AFAIK we only have two (very detailed and informative!) formal reviews so
far, which I fear is not enough to establish community consensus for
acceptance. I want to encourage more reviews from the community. If anyone
would like to extend the review deadline to give himself or herself more
time to look at the library, discuss it, and/or write a review, please let
me know!

Thanks,

- Jeff

(Original review announcement below.)

On Fri, Jun 8, 2012 at 7:28 AM, Jeffrey Lee Hellrung, Jr. <
jeffrey....@gmail.com> wrote:
[...]

[Edward Diener]

On 6/16/2012 3:08 PM, Jeffrey Lee Hellrung, Jr. wrote:
> The review of Multiprecision is due to end on Sunday, June 17th, 2012, but
> AFAIK we only have two (very detailed and informative!) formal reviews so
> far, which I fear is not enough to establish community consensus for
> acceptance. I want to encourage more reviews from the community. If anyone
> would like to extend the review deadline to give himself or herself more
> time to look at the library, discuss it, and/or write a review, please let
> me know!

I would like to see the review period extended by at least another full
week.

[John Maddock]

> The review of Multiprecision is due to end on Sunday, June 17th, 2012, but
> AFAIK we only have two (very detailed and informative!) formal reviews so
> far, which I fear is not enough to establish community consensus for
> acceptance. I want to encourage more reviews from the community. If anyone
> would like to extend the review deadline to give himself or herself more
> time to look at the library, discuss it, and/or write a review, please let
> me know!

Relating to that it would be useful to know if the lack of reviews is due
to:

1) No time.
2) No interest.
3) Don't like the library but are too polite to say so ;-)
4) Doesn't address the specific use case you're interested in.

Many thanks, John.

[John Maddock]

Vicente,

Many thanks for the review, some specific comment follow...

>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+().

The operators are defined as non-members.

>I would expect the result of binary operator-() always signed.

That's probably possible, although it makes the behaviour noticably
different from builtin integers.

>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'll do what I can do in this area, but the code is already pretty complex
and I worry about it becoming untestable, as well as the issues in deciding
what the "right thing to do" is for mixed arithmetic.

>* I would expect the library provide a back-end for representing N-bits
>integers with exactly (N/8)+1 bytes or something like that.
>
>* And also an overflow aware back-end for fixed precision integers.

OK, I'll see what I can do.

>* 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.

It's there already - the backend is always fed an unsigned value, and
mp_number only range checks the value provided if it's signed.

>* noexcep:
>
>The library interface should use the noexcept (BOOST_NOEXCEPT, ...)
>facilities.

There are some interfaces where that's trivial (for example the operator
overloads returning an expression template). For mp_number's member
functions, I don't know enough about noexcept in non trivial situations to
know if it's possible (can a template member function be noexcept when
whether it throws or not, depends on a dependant type?) There are also
plenty of functions which should be noexcept as they only call external C
library functions, but probably can't be marked as such without modifying
third party headers (gmp.h for example).

>* 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.

I think the functions that return an expression template can be constexp.
Will investigate other uses.

>* 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.

Again I don't yet know enough about constexp in non-trivial use cases to
know if we can do this. Will investigate.

>Adding a move constructor from the backend maybe could help.
>
>constexpr mp_number<BE>::mp_number(const BE&&);

Will add.

>Boost.Move should be used to emulate move semantics in compilers don't
>providing it.

Will investigate.

Regards, John.

[Christopher Kormanyos]

Thanks for your detailed and clear review, Vicente.

> 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 fixed-point arithmetic
> (as least as I understand it).

We can extend the docs, also as per your list in your review.

<snip>

> 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.

This is something that I would really like to work---even if fixed-point
is slow in multiprecision. It helps prove the concept of generic mathematical
programming in C++. It would also show that we boosters are act as
a coherent development community by attempting to obtain uniform
treatment for non-built-in types.

Perhaps we can work together and get your requirements done
so you can plug your future fixed-point into multiprecision.

Thanks again.

Best regards, Chris.

[Christopher Kormanyos]

>> The review of Multiprecision is due to end on Sunday, June 17th, 2012, but
>> AFAIK we only have two (very detailed and informative!) formal reviews so
>> far, which I fear is not enough to establish community consensus for
>> acceptance. I want to encourage more reviews from the community. If anyone
>> would like to extend the review deadline to give himself or herself more
>> time to look at the library, discuss it, and/or write a review, please let
>> me know!

> I would like to see the review period extended by at least another full week.

Edward, would you have time to take place in the review then?

Thank you, Chris.

[Christopher Kormanyos]

>> The review of Multiprecision is due to end on Sunday, June 17th, 2012, but
>> AFAIK we only have two (very detailed and informative!) formal reviews so
>> far, which I fear is not enough to establish community consensus for
>> acceptance. I want to encourage more reviews from the community. If anyone
>> would like to extend the review deadline to give himself or herself more
>> time to look at the library, discuss it, and/or write a review, please let
>> me know!

> Relating to that it would be useful to know if the lack of reviews is due to:

> 1) No time.
> 2) No interest.
> 3) Don't like the library but are too polite to say so ;-)
> 4) Doesn't address the specific use case you're interested in.

> Many thanks, John.

I really did expect a better turnout. I suppose everyone is just busy.
Then again, the European Soccer Championship is also on.

Did we get the wrong name?
Did we focus too much on generics?
Did we fail to address bare metal types like UINT128?
Are big numbers just too much like school and doing homework?

If we got it wrong, please contribute to the process of getting it right.
We do, however, actually need to work together on a consensus.

Boost truly needs support for large integers, rationals and floats.

If there is anything I can do to reduce the administrative burden
such as providing more examples, etc., please let me know.

Best regards, Chris.

[Keith Burton]

>> The review of Multiprecision is due to end on Sunday, June 17th, 2012,
but
>> AFAIK we only have two (very detailed and informative!) formal reviews so
>> far, which I fear is not enough to establish community consensus for
>> acceptance. I want to encourage more reviews from the community. If
anyone
>> would like to extend the review deadline to give himself or herself more
>> time to look at the library, discuss it, and/or write a review, please
let
>> me know!
>
> Relating to that it would be useful to know if the lack of reviews is due
> to:
>
> 1) No time.
> 2) No interest.
> 3) Don't like the library but are too polite to say so ;-)
> 4) Doesn't address the specific use case you're interested in.

> Many thanks, John.

I have written extended integer classes which have been used in commercial
software.
I have read all the discussion on multiprecision

Therefore I am sure I am not qualified to review the submission in any
detail.
However if I have a vote , it is for acceptance.

p.s. Add to John's list :
5) Know enough to know how little one knows about the problem domain.

[Jeffrey Lee Hellrung, Jr.]

On Sat, Jun 16, 2012 at 8:38 PM, Edward Diener <eldi...@tropicsoft.com>wrote:

> On 6/16/2012 3:08 PM, Jeffrey Lee Hellrung, Jr. wrote:
>
>> The review of Multiprecision is due to end on Sunday, June 17th, 2012, but
>> AFAIK we only have two (very detailed and informative!) formal reviews so
>> far, which I fear is not enough to establish community consensus for
>> acceptance. I want to encourage more reviews from the community. If anyone
>> would like to extend the review deadline to give himself or herself more
>> time to look at the library, discuss it, and/or write a review, please let
>> me know!
>>
>
> I would like to see the review period extended by at least another full
> week.
>

Edward and others,

I got the green light from Ron Garcia to extend the review period until
next Sunday, June 24th, 2012. Hopefully this gives sufficient time for
additional members to submit formal reviews.

- Jeff

[Jeffrey Lee Hellrung, Jr.]

Sounds like you're being too humble! If you get a chance, despite your
comments above, I think we would be collectively interested in your
thoughts regarding the proposed Multiprecision library within the context
of your past experience with "big integer" types.

(As review manager, it's my job to do a little prodding here and there.)

- Jeff

[Belcourt, Kenneth]

On Jun 17, 2012, at 2:00 AM, John Maddock wrote:

>> The review of Multiprecision is due to end on Sunday, June 17th, 2012, but
>> AFAIK we only have two (very detailed and informative!) formal reviews so
>> far, which I fear is not enough to establish community consensus for
>> acceptance. I want to encourage more reviews from the community. If anyone
>> would like to extend the review deadline to give himself or herself more
>> time to look at the library, discuss it, and/or write a review, please let
>> me know!
>
> Relating to that it would be useful to know if the lack of reviews is due to:
>
> 1) No time.
> 2) No interest.
> 3) Don't like the library but are too polite to say so ;-)
> 4) Doesn't address the specific use case you're interested in.

I know very little about this domain and don't currently have a need for it, I've pinged a few coworkers who may be interested in this area. I have a couple of questions though, as one who runs tests, I am mildly curious how big the library footprint is (disk space, peak memory, compile times, runtimes, ...)? Many ET libraries can't compile in the 5 minutes nightly testing provides, will this be an issue for your library? Is the testing infrastructure setup to automatically locate dependent libraries (GMP, MPFR, ...) and enable those tests if found, or is this a manual configuration step?

-- Noel

[Edward Diener]

On 6/17/2012 6:16 AM, Christopher Kormanyos wrote:
>
>
>
>>> The review of Multiprecision is due to end on Sunday, June 17th, 2012, but
>>> AFAIK we only have two (very detailed and informative!) formal reviews so
>>> far, which I fear is not enough to establish community consensus for
>>> acceptance. I want to encourage more reviews from the community. If anyone
>>> would like to extend the review deadline to give himself or herself more
>>> time to look at the library, discuss it, and/or write a review, please let
>>> me know!
>
>> I would like to see the review period extended by at least another full week.
>
> Edward, would you have time to take place in the review then?

Yes.

[Edward Diener]

On 6/17/2012 4:00 AM, John Maddock wrote:
>> The review of Multiprecision is due to end on Sunday, June 17th, 2012,
>> but
>> AFAIK we only have two (very detailed and informative!) formal reviews so
>> far, which I fear is not enough to establish community consensus for
>> acceptance. I want to encourage more reviews from the community. If
>> anyone
>> would like to extend the review deadline to give himself or herself more
>> time to look at the library, discuss it, and/or write a review, please
>> let
>> me know!
>
> Relating to that it would be useful to know if the lack of reviews is
> due to:
>
> 1) No time.
> 2) No interest.

I am very interested in the library but have not had the time to try it
out. Next weekend I can take a number of hours to do so.

[Steven Watanabe]

AMDG

Here are my comments from reading through the tutorial.

Documentation:

Introduction:

* "precision may be arbitrarily large ..., fixed ..., or a variable ..."

You shouldn't combine adjective, adjective, noun
with "or" like this. The elements should be
the same part of speech.

* "An expression-template-enabled front-end mp_number"
"An expression-template-enabled front-end*,* mp_number"
A non-restrictive appositive should have a comma.

* "Separation of ... but provides Boost license alternatives ..."

The subject of "provides" is "separation," which doesn't
make a whole lot of sense.

* "Which is to say some back-ends rely on 3rd party libraries ..."

Is it really necessary to restate the previous sentence?

* "typedef mp::mp_number<mp::mpfr_float_backend<300> > my_float;"

Is there a reason to have two spaces before "my_float?"

* "Note that mixing arithmetic operations using types of
different precision is strictly forbidden:"

The example uses cpp_int, which hasn't been introduced
before. Does this rule apply to all backends? If
so it would flow better if you continued to use mpfr.
Also, if the rule is anything more complex than
"mixing different specializations of mp_number is
illegal," then I'd like to see a link to a more
detailed discussion of what kind of mixing is
legal and what kind of mixing isn't.

Expression Templates:

* "For example, lets suppose we're evaluating a polynomial via Horners
method..."
s/lets/let's/ s/Horners/Horner's/

* "... Horners method, something like this:"
This doesn't parse. "like," which could have introduced
a dependent clause, modifies "something," which doesn't
have any legitimate place in the sentence.

* "... the mpfr_class C++ wrapper for MPFR - then this expression ..."
Is there any particular reason to use a hypen here?
Normally, it should be a comma.

* "Had we used ... things would have been ... and no less that 24
temporaries are created"
s/that/than/
Also, don't mix moods and tenses like this.
(i.e. it should read "... temporaries would have been created")

* "... rather than the number of temporaries directly"
"directly" is an adverb, but it's being used to
modify a noun "number." You can fix this by inserting
a gerund: "rather than measuring the number of temporaries
directly."

* "... directly, note also that the mpf_class ..."
Run on sentence.

* "Finally if we use this library with "
Comma after Finally.

* "Of course, should you write:

x = sin(x);

Then we clearly ..."

"Then" shouldn't be capitalized, because it only
introduces an independent clause, not a new
sentence. (There are many other instances of this.)

* "... during the calculation, so then a temporary variable ..."
Delete "then." It sounds awkward to me.

* "... that expression-templates are some kind of universal-panacea"
"universal-panacea" doesn't need a hyphen, since
it's just an adjective modifying a noun, not a
compound word. Not to mention that "universal"
is redundant.

* "than their simpler cousins, they're also harder"
Run on sentence.

* "Having said that, these situations don't occur that often -
or indeed not at all for non-template functions"

There are way too many negatives in this sentence.

* "that the lifetimes of a, b and c will outlive that"
Missing comma before "and".

* "... multiplication, where as operator* can use the target ..."
"where as" doesn't make sense. Try "but"

* "Even so the transformation is more efficient than creating
the extra temporary variable"
There should be a comma after "Even so".
The transformation is not more efficient than anything.
I think you mean that the transformation makes the
/transformed code/ more efficient.

* "... argument, which, when set to false disables all the ..."
There should be a comma after "false."

* "... between these three libraries, again, all are using ..."
Run on sentence.


General notes:

* There are a lot of places where a feel like
an article would make the text read more
smoothly.
* Please decide whether you want sentences
starting with "For example", "Instead", etc.
to have a comma. You aren't consistent.
* Maybe try to use fewer gerunds. It's starting
to annoy me.

Tutorial:

Integer Types:

* "Very versatile, Boost licensed, all C++ integer
type which support both "

s/support/supports/

* "Very fast and efficient back-end."
The redundancy doesn't do anything for me.

cpp_int

* "When the Allocator parameter is type void, then this field"
"then" should really only be used with "if"

* "Note that for arbitrary precision types ..., then this parameter ..."
Remove "then"

* "For fixed precision types then this type ..."
Remove "then"

* "Default constructed cpp_int_backends have the value zero."
So far you've only used mp_number<cpp_int_backend<...> >
and the relationship between default constructing
cpp_int_backend and constructing mp_number has
not been specified.

* "In might be tempting to use a 127-bit type instead ..."
s/In/It/

* ".. identical (apart from the sign), where as they ..."
"where as" doesn't make sense. Try "although."

* "Attempting to print negative values as either an Octal
or Hexadecimal string ..."
Mismatched number. (values vs. a string)

* "...a std::runtime_error being thrown, this is a direct consequence ..."
Run on sentence.

gmp_int:

* "It's also possible to access the underlying mpz_t via the
data() member function of gmp_int"
I'm assuming from this that mpz_int ends up inheriting
data() from gmp_int? Otherwise, this wouldn't be
terribly useful. Anyway, (I had a similar comment
for cpp_int) I thing you should either specify everything
in terms of the appropriate specializations of mp_number
or say somewhere early on that mp_number inherits
all the behavior of the backend. Wait... I see v.backend().data()
in the example.

* ".... notation for negative values, as a result performing formatted ..."
Run on sentence.

tom_int

* "... for the builtin integer types, it should be noted ..."
Run on sentence.

* "it is a rather strange beast as it's a signed type that is
not a 2's complement type"
isn't this true of GMP and cpp_int as well?
Why the special warning for tom_int?

Examples:

Factorials:

Bit Operations:

* "... integer may be manipulated, we'll start with an often ..."
Run on sentence.

* "... just set the n'th bit in the ..."
Oh, come on. I'm sure there's a way to use a proper superscript.

Floating Point Numbers:

* I'd like to see the same order as in Integer Types.
(i.e. cpp_dec_float before GMP).

gmp_float:

* "... overflow or division by zero. That latter will trigger ..."
s/That/The/

mpfr_float:

cpp_dec_float:

* "There is full standard library and numeric_limits
support available for this type."
It's better to avoid "there is." It just adds verbosity.
i.e. "Full ... support is available...." or use the
active voiceS "This type has full support for ..."

* On a related note, what does "standard library support"
mean? ... In the example, I see "log(b)." I think
this terminology is imprecise. log(b) does not
call a standard library function at all.

* "Narrowing conversions are truncating."
This is unfortunate. I'd prefer it to round according
to the current rounding mode.

Examples:

Area of Circle:

Defining a Lambda Function:

* The name of the section is misleading because
"Lambda" has a totally different meaning in
programming. Maybe find a different example.

* "Now lets implement the ..."
s/lets/let's/

* "... non-template examples, lets repeat the code ..."
s/lets/let's/

* "...mixed-argument functions, here's how the new ..."
Run on sentence.

* I'm a little concerned that if the arguments of the
final version, JEL5 are expression templates, they
would be evaluated multiple times. Is this the
case?

Calculating a Derivative:

Calculating an Integral:

* "Similar to the ... in a similar manner"
Don't use similar twice.

* You should probably explain that the algorithm
uses the trapezoid rule and iteratively cuts
the step size in half.

* It would be nice if there were a way to specify the
minimum number of steps. Otherwise, it's tricky
in cases like: \int_0^{2\pi} sin(x) dx. (Of course,
this is only example code...)

* "... how the function can be called, we begin by defining ..."
Run on sentence.

Polynomial Evaluation:

Rational Number Types:

gmp_rational:

cpp_rational:

tommath_rational:

Use With Boost.Rational:

rational_adapter:

* class rational_adpater;
typo.

Constructing and Interconverting Between Number Types:

* "Any number type will interoperate with the
builtin type in arithmetic expressions:"
There's more than one builtin type.
Maybe "any builtin arithmetic type?"

* "An mp_number can be converted to any built in type,
via the convert_to member function"
In C++11, can we use an explicit conversion operator?

Generating Random Numbers:

* boost/multiprecision/random.hpp should go away.
I'll try to work out the changes needed to
make Boost.Random work directly.

Primality Testing:

* "... test for primality, if the result is false ..."
Run on sentence.

* "... when producing random primes then you should ..."
Remove "then".

In Christ,
Steven Watanabe

[John Maddock]

> I know very little about this domain and don't currently have a need for
> it, I've pinged a few coworkers who may be interested in this area.
>I have a couple of questions though, as one who runs tests, I am mildly
>curious how big the library footprint is (disk space, peak memory, compile
>times,
>runtimes, ...)? Many ET libraries can't compile in the 5 minutes nightly
>testing provides, will this be an issue for your library? Is the testing
>infrastructure setup to
>automatically locate dependent libraries (GMP, MPFR, ...) and enable those
>tests if found, or is this a manual configuration step?

Both compile times and runtimes are an issue, that said I am aware of the
testing limitations and have tried to live within them - exactly how
successful that will prove to be, will no doubt become apparent if the
library heads off to the test runners! As for dependencies - those should
be found automatically if they're in the compiler's search paths already.

John.

[John Maddock]

> Here are my comments from reading through the tutorial.

Many thanks for the detailed comments.

> * "It's also possible to access the underlying mpz_t via the
> data() member function of gmp_int"
> I'm assuming from this that mpz_int ends up inheriting
> data() from gmp_int? Otherwise, this wouldn't be
> terribly useful. Anyway, (I had a similar comment
> for cpp_int) I thing you should either specify everything
> in terms of the appropriate specializations of mp_number
> or say somewhere early on that mp_number inherits
> all the behavior of the backend. Wait... I see v.backend().data()
> in the example.

Right, the backend type is accessible via the backend() member function.

> * "it is a rather strange beast as it's a signed type that is
> not a 2's complement type"
> isn't this true of GMP and cpp_int as well?
> Why the special warning for tom_int?

Lib tommath's bitwise operations behave differently from a 2's complement
type. In contrast gmp and cpp_int "simulate" 2's complement behaviour when
performing bitwise ops on negative numbers. Note that this is an area where
the behaviour of builtin types is undefined in the std anyway, there is
however, a general expection that they will actually behave in a certain
way.

> * "... just set the n'th bit in the ..."
> Oh, come on. I'm sure there's a way to use a proper superscript.

I'm not sure I follow, do you mean make mp_number subscriptable to access
the individual bits? Obviously that's possible to do, I'd be interested in
others thoughts as to whether it's the correct interface choice.

> * On a related note, what does "standard library support"
> mean? ... In the example, I see "log(b)." I think
> this terminology is imprecise. log(b) does not
> call a standard library function at all.

I'll try and be more specific - it means that all the functions (from cmath)
and traits (numeric_limits) that operate on builtin floating point types,
are also defined for these types.

> * "Narrowing conversions are truncating."
> This is unfortunate. I'd prefer it to round according
> to the current rounding mode.

Chris will have to answer that one.

> Defining a Lambda Function:
>
> * The name of the section is misleading because
> "Lambda" has a totally different meaning in
> programming. Maybe find a different example.

Point taken, shame, it made a nice simple example :-(

> * I'm a little concerned that if the arguments of the
> final version, JEL5 are expression templates, they
> would be evaluated multiple times. Is this the
> case?

My bad, yes that would be the case. Will fix the example.

> * "An mp_number can be converted to any built in type,
> via the convert_to member function"
> In C++11, can we use an explicit conversion operator?

Not yet, but's already ben asked for and trivial to add.

> Generating Random Numbers:
>
> * boost/multiprecision/random.hpp should go away.
> I'll try to work out the changes needed to
> make Boost.Random work directly.

I hoped you'd say that ;-)

However, I've deliberately held off submitting patches or suggesting any
changes until/unless the interface on this library is accepted. No point in
chasing a moving target. Just consider the code in random.hpp proof of
concept for now, I figure folks would complain about it not working if I
didn't provide it.

Regards, John.

[Christopher Kormanyos]

>>>> The review of Multiprecision is due to end on Sunday, June 17th, 2012, but
>>>> AFAIK we only have two (very detailed and informative!) formal reviews so
>>>> far, which I fear is not enough to establish community consensus for
>>>> acceptance. I want to encourage more reviews from the community. If anyone
>>>> would like to extend the review deadline to give himself or herself more
>>>> time to look at the library, discuss it, and/or write a review, please let
>>>> me know!

>>> I would like to see the review period extended by at least another full week.

>> Edward, would you have time to take place in the review then?

> Yes.

Thank you, Edward.

[Christopher Kormanyos]

> Here are my comments from reading through the tutorial.

> Documentation:

<snip Steven's comments on docs>

> In Christ,
> Steven Watanabe

Thank you for your detailed comments and suggestions, Steven.

John, should I try to look into Steven's documentation issues during
the upcoming weekend or would you prefer to keep going on that?

<snip>

Best regards, Chris.

[Christopher Kormanyos]

<snip>

>> * "Narrowing conversions are truncating."
>>  This is unfortunate.  I'd prefer it to round according
>>  to the current rounding mode.

> Chris will have to answer that one.

Yes. I agree. I would also prefer cpp_dec_float to round
when converting to lesser digits or built-in types. Unfortunately,
though, cpp_dec_float in its current (*well-tested*) state does not
support rounding. It only rounds to nearest when creating an output
string.

The cpp_dec_float class uses an internal base-10 representation.
This has some advantages such as providing an intuitive form allowing
for ease of visualization with a debugger. There are, however, unfortunate
drawbacks with a base-10 representation. In particular conversions
to and from base-10 to base-2 representations are difficult to implement.

Several reviewers have been disappointed with my back-end.
(That's a funny sentence, isn't it?)

By that I mean disappointed in the lack of rounding thereof.

If Boost.Multiprecision gets accepted and if I can find a reliable,
efficient way to round to nearest when performing narrowing
conversions with cpp_dec_float, then I will implement this rounding.
Brute force conversion to string would be the last resort,
but atrociously inefficient---not my first choice.

John, could you please add this to the ToDo list?

<snip>

>> Defining a Lambda Function:
>>
>> * The name of the section is misleading because
>>  "Lambda" has a totally different meaning in
>>  programming.  Maybe find a different example.

> Point taken, shame, it made a nice simple example :-(

The full name of the function is: Jahnke-Emden-Lambda Function.
Maybe this is enough for the documentation.

Alternatively, there is also the classic "sinc" function defined as

sinc(x) = sin(x) / x

Calculating the sinc function uses a very simple strategy:
Taylor series for small arguments and straight forward evaluation
for large arguments. The example would, therefore, remain terse
and east to understand.

I could probably come up with another pithy, witty example.
I would, however, need more time to conjure up one.

Best regards, Chris.

[John Maddock]

> Thank you for your detailed comments and suggestions, Steven.
>
> John, should I try to look into Steven's documentation issues during
> the upcoming weekend or would you prefer to keep going on that?

If you've got some time, then yes by all means go for it!

Thanks, John.

[Christopher Kormanyos]

>> Thank you for your detailed comments and suggestions, Steven.
>>
>> John, should I try to look into Steven's documentation issues during
>> the upcoming weekend or would you prefer to keep going on that?

> If you've got some time, then yes by all means go for it!
> Thanks, John.

OK. I'll try to do it.

Best regards, Chris.

[Marc Glisse]

Hello,

I just found this unsent draft. I don't remember what else I wanted to
say...

On Sun, 17 Jun 2012, John Maddock wrote:

>> * noexcep:
>>
>> The library interface should use the noexcept (BOOST_NOEXCEPT, ...)
>> facilities.
>
> There are some interfaces where that's trivial (for example the operator
> overloads returning an expression template). For mp_number's member
> functions, I don't know enough about noexcept in non trivial situations to
> know if it's possible (can a template member function be noexcept when
> whether it throws or not, depends on a dependant type?)

noexcept(noexcept(expression)) works great.

> There are also
> plenty of functions which should be noexcept as they only call external C
> library functions, but probably can't be marked as such without modifying
> third party headers (gmp.h for example).

GMP functions that are noexcept are marked as such. Functions that may
allocate may throw and thus can't be noexcept (although a macro that says:
"assume there is enough memory" might make sense).

>> Adding a move constructor from the backend maybe could help.
>> constexpr mp_number<BE>::mp_number(const BE&&);

I am not sure I understand what that's good for?

Just to mention a few other things. For mixed arithmetic, having
operations like:
number<common_type<A,B>::type> operator+(number<A>,number<B>) that call
eval_add(common_type<A,B>::type&,A,B) seems good. If A==B, it doesn't
change anything. If a backend is implicitly convertible to another,
operations just work. And the backend writer can still overload eval_add
to optimize some operations. You could even consider turning that into a
result_trait<T...,op> that defaults to common_type<T...>::type but can be
specialized by backend writers and determines mp_exp<...>::result_type (I
notice you already have combine_expression that could be extended).

Having the public type (mp_number) and the expression type (mp_exp) be
specializations of the same template could save a few lines of code?

You seem to go further than I did in gmpxx to remove temporaries by
reusing the result: you check for aliasing within any expression, whereas
I stopped at depth one (I had a prototype for arbitrary depth, and it
simplified the code, but I was scared of the potential quadratic cost, I
probably shouldn't be). Although I am surprised that in a=expr1+expr2,
when a appears in expr1 but not expr2, you don't try to take advantage of
it (not that it would help much now, but it might if you eventually add
FMA).


Good luck with the reviews,

--
Marc Glisse

[Christopher Kormanyos]

>> Thank you for your detailed comments and suggestions, Steven.
>>
>> John, should I try to look into Steven's documentation issues during
>> the upcoming weekend or would you prefer to keep going on that?

> If you've got some time, then yes by all means go for it!
> Thanks, John.

I worked over the documentation based on Steven's suggestions
and  incorporated some editing of my own---primarily in the
first portions of the text. There can be further improvements.
For example, I suggest to adding a table, which is in TBD-form
in the text.

John, with your permission, may I commit the changes
to the documentation?

When building the documentation, I only get the XML target.
I am such a beginner, it hurts! Is the warning about "standalone"
below important?

warn: Unable to construct ./standalone
warn: Unable to construct ./standalone
...patience...
...patience...
...found 1705 targets...
...updating 1 target...
quickbook.quickbook-to-boostbook bin\msvc-10.0\debug\big_number.xml
Generating Output File: bin\msvc-10.0\debug\big_number.xml
...updated 1 target...

Best regards, Chris.

[John Maddock]

>> Thank you for your detailed comments and suggestions, Steven.
>>
>> John, should I try to look into Steven's documentation issues during
>> the upcoming weekend or would you prefer to keep going on that?

> If you've got some time, then yes by all means go for it!
> Thanks, John.

>John, with your permission, may I commit the changes
>to the documentation?

Sure - but please check the built html look OK first ;-)

>When building the documentation, I only get the XML target.
>I am such a beginner, it hurts! Is the warning about "standalone"
>below important?

Yup. Do you have xsltproc and docbook configured in your user-config.jam?
If not please see:
https://svn.boost.org/trac/boost/wiki/BoostDocs/GettingStarted (note I've
never tried this with the cygwin toolchain, working with the Win32 binaries
works OK though).

HTH, John.

[Marc Glisse]

On Sat, 23 Jun 2012, Marc Glisse wrote:

> I don't remember what else I wanted to say...

Ah, I probably wanted to mention the possibility to reuse the same
temporaries in 2 subtrees. When evaluating a=expr1+expr2, expr1 and expr2
are evaluated independently and may both create temporaries. Having them
share these temporaries is possible but complicated enough that I haven't
bothered yet.

[Keith Burton]

Instant reactions on reading though the documentation :

1) Introduction : I would prefer to see e.g. mp::int128_t rather than
mp::mp_int128_t, given the stated purpose of the library.

2) cpp_dec_float : Narrowing conversions are truncating. This is a no-no
for the usage I have in mind at the moment.

3) Constructing and interconverting :

>> // There is a need to be careful with integer types though:
>> cpp_int i = 2;
>> // Ooops, this really just multiplies by 3:
>> i *= 3.14;

One of the priorities for my numeric class implementations has always been
to protect naive users from equivalent mistakes.

What is your evaluation of the design?

It is so far beyond anything I have attempted that I cannot usefully
comment. The limitations on conversions and mixed precision operations are
disappointing but entirely understandable. The ccp_dec_float narrowing
conversions being truncating reinforces the my view that the limitations are
probably the correct design.

What is your evaluation of the implementation?

No useful comments.

What is your evaluation of the documentation?

Good enough to give me confidence to start using the library. I
would never expect more than that.

What is your evaluation of the potential usefulness of the library?

A vital addition to C++ especially for relatively small tasks in
commercial software where the Boost backends can be used because performance
is not an overriding concern.

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

I have started to use the library to embed test code for my special
purpose decimal fixed point classes into an application under development.
The only issue, using VS2008 /W4 and Boost 1.48, was the need to
disable warning 4996.
Unfortunately this has been pre-empted by other work.

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

More than a quick reading

Are you knowledgeable about the problem domain?

I have written several numeric classes but all with a narrow scope
and protection of naïve users and accuracy as the priority.
These were primarily decimal fixed point.

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

Yes

[Marc Glisse]

On Sat, 23 Jun 2012, Marc Glisse wrote:

>> I don't remember what else I wanted to say...

One other thing: I am not sure where it is said in the doc that the
expression template mechanism reassociates operations (turning a+(b+c)
into (a+b)+c), which may be undesirable for some floating point backends.
Turning the bool that determines the use of expression templates into an
enum with more details might be too painful, but adding a few sentences to
the doc would already help.

[John Maddock]

Many thanks for the review,

>1) Introduction : I would prefer to see e.g. mp::int128_t rather than
>mp::mp_int128_t, given the stated purpose of the library.

OK.

>2) cpp_dec_float : Narrowing conversions are truncating. This is a no-no
>for the usage I have in mind at the moment.

I know, it's something we need to at least try and fix.

>3) Constructing and interconverting :

>> // There is a need to be careful with integer types though:
>> cpp_int i = 2;
>> // Ooops, this really just multiplies by 3:
>> i *= 3.14;

>One of the priorities for my numeric class implementations has always been
>to protect naive users from equivalent mistakes.

Nod.

We have a request already for these conversions to be explicit, the next
step is to try and disallow the operator overloads (enable_if) for narrowing
conversions.

>I have started to use the library to embed test code for my special
>purpose decimal fixed point classes into an application under development.
>The only issue, using VS2008 /W4 and Boost 1.48, was the need to
>disable warning 4996.

Nod. We only use C++ std lib functions, but MS in their wisdom have
"depricated" some of them :-(

Thanks again, John.

[Matthieu Schaller]

Here is my late review. Apologies for the delays.

What is your evaluation of the design?

Boost.Multiprecision is interesting as it brings boost into area of
computer science where it is not very present yet. Providing a high-level
fixed/arbitrary precision integer/float type library to numerical scientist
is a great move.

The library is easy to use and behaves as one could expect when looking at
the function name or operator. One can thus easily move any mathematical
calculation from a usual POD double to a cpp_dec_float or mpf_float. A
beginner will understand rapidly what a given piece of code using this
library does.
I especially like the wrapper around GMP which is a standard among
numerical scientists.

The conversions are however sometimes tricky to understand but I do not
know if there is a solution to this issue which would satisfy everyone in
every situation.

What is your evaluation of the implementation?

As already mentioned, I really like the ability to use pre-existing
libraries such as GMP which are well-known and well-tested. The speed
difference is really minimal and is probably worth the comfort.
All functions do what you expect and the list of supported functions and
constants is really comprehensive. Everyday use should be a problem. The
support of numeric_limits is also a good thing for re-use in other
libraries.

The decomposition in front-end and back-end is particularly good for future
extensions.

The optional use of expression template is a great feature. The proposed
improvements present on the TODO list aiming at improving the speed of the
implementation will reduce the difference between cpp_int and the GMP
equivalent even further.

What is your evaluation of the documentation?

The documentation is really comprehensive and presents interesting
examples. The tricky points (conversions, ... ) are discussed and a
beginner should be able to write a code without loosing his time reading an
obscure documentation

What is your evaluation of the potential usefulness of the library?

Useful to anyone needing high precision mathematical computations.

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

Used with GCC 4.6 and ICC 12.1 to test my own library proposal (complex
numbers). No problems nor warnings.
The results obtained always agreed with the values given by Maple.
As mentioned by others, one must still be careful with conversions. But any
user of extended precision libraries should be aware of these caveats
anyway. Furthermore, these are well documented.

It was easy to use the cpp_dec_float or gmp_float classes in the context of
complex numbers.

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

Good effort at understanding and testing most of the floating point
library in the context presented above. Tested the integer types for this
review.

Are you knowledgeable about the problem domain?

Yes. See the extended complex number class proposal.

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

If boost wants to move into the realm of extended precision numbers, then
yes.

Lastly, please consider that John and Christopher have compiled a TODO list
[1] based on pre-review comments. Feel free to comment on the priority and
necessity of such TODO items, and whether any might be show-stoppers or
warrant conditional acceptance of the library.


In my opinion, these are not show-stoppers. Adding more back-ends is always
good but the main library in this field (GMP) can already be used. So this
should not be a reason to reject this library.
The same is true with the possible improvements to cpp_int_backend. Users
wanting a really fast code can use the GMP back-end. Users in the real
world may also make suggestions for other possible improvements or express
their needs.
Having a version of this library in the real world is a good thing to spot
its possible weaknesses and will also allow the authors to work on the
elements of their TODO lists.

Regards,

Matthieu

--
Matthieu Schaller

[Steven Watanabe]

AMDG

On 06/24/2012 04:25 PM, Matthieu Schaller wrote:
>
> Do you think the library should be accepted as a Boost library?
>
> If boost wants to move into the realm of extended precision numbers, then
> yes.
>

What do you mean "If boost want to...?" There is
no way to determine this apart from the review process,
/which you are a part of by submitting a review/.

In Christ,
Steven Watanabe

[Christopher Kormanyos]

Thank you for your review, Keith.

> 1) Introduction : I would prefer to see e.g. mp::int128_t rather than
> mp::mp_int128_t, given the stated purpose of the library.

Yes, the consensus seems to be that the additional "mp_" part
of the class names is redundant.

2) cpp_dec_float : Narrowing conversions are truncating.  This is a no-no
for the usage I have in mind at the moment.

Yes, almost all reviewers requested rounding as an improvement
over truncation. This is one of my parts of the library, so i might
need to work on it and maybe seek help from others, since I don't
know a clever way to round radix-10 to radix-2.

<snip>

Best regards, Chris.

[Edward Diener]

On 6/8/2012 10:28 AM, Jeffrey Lee Hellrung, Jr. wrote:
>
> Any review discussion should take place on the developers' list (
> bo...@lists.boost.org), and anyone may submit a formal review, either
> publicly to the entire list or privately to just myself.
>
> As usual, please consider the following questions in your formal review:

This is my review of the multiprecision library.

I am going to use the term "general type" to refer to either 'integer',
'rational number', or 'floating point' type no matter what the backend is.

>
> What is your evaluation of the design?

The design is logical. I like the fact that various backends are
supported and that there is always a Boost backend to fall back on for a
general type. I also like it very much that future backends are also
supported via the backend requirements.

> What is your evaluation of the implementation?

I looked the implementation from the point of view as an end-user but
did not look at the details. I did not have the chance to test the
implementation so my comments are based on the doc.

It seems very easy to use the implementation. The hardest part is that
some of the backends have their own rules, which are not entirely
consistent with other backends of the same general type ( integer,
rational number, or floating point ). This does not occur very often but
when it does the end-user has to be aware of it. These slight
inconsistencies are documented but I would like to see an attempt to
regularize them by the front-end, perhaps via a compile time trait.

As a single example of this the gmp_int backend triggers a division by 0
signal when one tries to divide the integer by 0; the tom_int raises a
hardward signal with a division by 0; the cpp_int throws a
std::runtime_error with a division by 0. I would like to see some means
by which I could use any integer backend and know that a
std::runtime_error would be thrown by a division by 0.

It would be nice if other partcularities could be regularized in a
similar manner. This would make it little easier to use the library
generically.

The library allows conversions between values of a general type no
matter what the backend. This is good. The library allows what I think
of as widening conversions, from integer to rational or float, and from
rational to float. This is good. Both follow the similar idea in C++
itself. In C++ one can do a narrowing conversion if a static_cast is
used, otherwise a compiler error ensues. The document also explains that
a narrowing conversion will produce a compiler error.

Can a static_cast be used to do a narrowing conversion ?

The introduction states that "mixing arithmetic operations using types
of different precision is strictly forbidden". I was disappointed not to
read any discussion of why this would be so. In C++ this is not the case
with integer and floating point types. Considering that this library
does allow conversons within the same general type and widening
conversions it would seem that doing operations with different types
could be technically allowed fairly easily, by converting all values in
an operation to the largest type and/or greatest precision.

> What is your evaluation of the documentation?

The doc is totally adeqate.

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

Tremendously important to C++. Although I am not a mathematician myself,
my background in math and science suggests that a multiprecision library
involving huge and/or highly accurate numbers is an absolute necesssity
to serious mathematical and scientific calculations.

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

I did not have time to try it out with any compiler. I originally wanted
to modify some of the tests in the library so I could try them out with
the compilers I have, but the tests were too complicated for me to
understand easily. I am going to try to cobble together some simple
tests for myself during the upcoming week but i wanted to submit my
review nonetheless before the period for it was over.

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

I put a good deal of effort attempting to understand the pluses and
negatives of the design, and thinking about the problems involved.

> Are you knowledgeable about the problem domain?

I am fairly knowledgeable about mathematics without being a
mathematician or claiming expertise in any particular area of mathematics.

>
> And, most importantly, please explicitly answer the following question:

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

I think the library should be accepted as a Boost library. I do think
more work needs to be done to make the library usable, but I have no
doubt the authors of the library are capable of doing so.

>
> Lastly, please consider that John and Christopher have compiled a TODO list
> [1] based on pre-review comments. Feel free to comment on the priority and
> necessity of such TODO items, and whether any might be show-stoppers or
> warrant conditional acceptance of the library.

I would like to see more work done in the two areas I mentioned:
regularizing the backends and performing operations with different
precisions. I do realize that accuracy is paramount when using the
library, but in the tradition of C++ as long as the end-user knows any
possible shortcoming of these two areas I think they should be allowed
if technically feasible.

Eddie Diener

[Steven Watanabe]

AMDG

On 06/24/2012 07:58 PM, Edward Diener wrote:
>
> As a single example of this the gmp_int backend triggers a division by 0
> signal when one tries to divide the integer by 0; the tom_int raises a
> hardward signal with a division by 0; the cpp_int throws a
> std::runtime_error with a division by 0. I would like to see some means
> by which I could use any integer backend and know that a
> std::runtime_error would be thrown by a division by 0.
>

IMO, you should just avoid division by zero period.
As far as C++ itself is concerned, division by 0
is undefined behavior, so any code that wants to
handle any numeric type including builtins can't
assume anything about division by zero.

In Christ,
Steven Watanabe

[Steven Watanabe]

AMDG

I really haven't had time to go through this
library as thoroughly as I would have liked,
but, since it's the last day of the review:
*I vote to accept Multiprecision into Boost*

Here are my comments on the code so far.
It's not very much, since I only got
through a few files. I'll try to expand
on this as I have time, but no promises.

Revision: sandbox/big_number@79066

concepts/mp_number_architypes.hpp:

* The file name is misspelled. It should
be "archetypes."

* The #includes aren't quite right. The
file does not use trunc, and does use
fpclassify.

* line 28: mpl::list requires #include <boost/mpl/list.hpp>
* line 79: stringstream requires #include <sstream>

depricated:

* Should be deprecated.

detail/functions/constants.hpp:

* line 12: typedef typename has_enough_bits<...> pred_type;
If you're going to go through all this trouble to make
sure that you have a type with enough bits, you shouldn't
assume that unsigned has enough. IIRC, the standard
only guarantees 16 bits for unsigned.

* line 38: if(digits < 3640)
Ugh. I'm assuming that digits means binary digits
and 3640 = floor(log_2 10^1100)

* line 51: I would appreciate a brief description
of the formula. This code isn't very readable.

* line 105: It looks like this is using the formula
e \approx \sum_0^n{\frac{1}{i!}} = (\sum_0^n\frac{n!}{i!})/n!
and induction using the rule:
\sum_0^n\frac{n!}{i!} =
(\sum_0^(n-1)\frac{(n-1)!}{i!}) * n + 1

Please explain the math or point to somewhere
that explains it.

* line 147: ditto for pi

* line 251: calc_pi(result, ...digits2<...>::value + 10);
Any particular reason for +10? I'd prefer
calc_pi to do whatever adjustments are
needed to get the right number of digits.

detail/functions/pow.hpp:

* line 18 ff: pow_imp
I don't like the fact that this implementation
creates log2(p) temporaries on the
stack and multiplies them all together
on the way out of the recursion. I'd
prefer to only use a constant number
of temporaries.

* line 31:
I don't see how the cases > 1 buy you anything.
They don't change the number of calls to eval_multiply.
(At least, it wouldn't if the algorithm were
implemented correctly.)

* line 47:
???: The work of this loop is repeated at every
level of recursion. Please fix this function.
It's a well-known algorithm. It shouldn't be
hard to get it right.

* line 76: pow_imp(denom, t, -p, mpl::false_());
This is broken for p == INT_MIN with twos-complement
arithmetic.

* line 185: "The ldexp function..."
I think you mean the "exp" function.

* line 273: if(x.compare(ll) == 0)
This is only legal if long long is in T::int_types.
Better to use intmax_t which is guaranteed to be in
T::int_types.

* line 287: const bool b_scale = (xx.compare(float_type(1e-4)) > 0);
Unless I'm missing something, this will always be
true because xx > 1 > 1e-4 as a result of the
test on line 240.

* line 332: if(&arg == &result)
This is totally unnecessary, since you already copy
arg into a temporary using frexp on line 348.

* line 351: if(t.compare(fp_type(2) / fp_type(3)) <= 0)
Is the rounding error evaluating 2/3 important?
I've convinced myself that it works, but it
took a little thought. Maybe add a comment to
the effect that the exact value of the boundary
doesn't matter as long as its about 2/3?

* line 413: "The fabs function "
s/fabs/log10/.

* line 464: eval_convert_to(&an, a);
This makes the assumption that conversion
to long long is always okay even if the
value is out of the range of long long. This
assumption makes me nervous.

* line 465: a.compare(an)
compare(long long)

* line 474:
eval_subtract(T, long long)
Also, this value of da is never used. It
gets overwritten on line 480 or 485.

* line 493: (eval_get_sign(x) > 0)
This expression should always be true at this point.

* line 614: *p_cosh = x;
cosh is an even function, so this is wrong when
x = -\inf

* line 625: T e_px, e_mx;
These are only used in the generic implementation.
It's better to restrict the scope of variables
to the region that they're actually used in.

* line 643: eval_divide(*p_sinh, ui_type(2));
I notice that in other places you use ldexp for
division by powers of 2. Which is better?

detail/mp_number_base.hpp:

* line 423: (std::numeric_limits<T>::digits + 1) * 1000L
This limits the number of bits to LONG_MAX/1000.
I'd like there to be some way to know that as
long as I use less than k bits, the library
won't have any integer overflow problems. k
can depend on INT_MAX, LONG_MAX, etc, but the
implementation limits need to be clearly documented
somewhere. I really hate the normal ad hoc
situation where you just ignore the problem and
hope that all the values are small enough to avoid
overflow.

Other notes:

Backend Requirements:

* Do we always require that all number types
be assignable from intmax_t, uintmax_t,
and long double? Why must an integer
be assignable from long double?

* The code seems to assume that it's okay to
pass int as the exp parameter of ldexp.
These requirements don't mandate that this
will work.

[Marc Glisse]

A couple more remarks (sorry for sending comments as they come):

- is mp_int128_t an alias for __int128 when gcc provides it? (or for long
long on platforms where it is 128 bit long)

- boost contains a library (proto) for writing expression templates. Not
eating your own dog food always sends a strange signal. Maybe a couple
words in the doc could help with that?

[Christopher Kormanyos]

> Here is my late review. Apologies for the delays.

Thank you for your review, Matthieu.

> The optional use of expression template is a great feature. The proposed
> improvements present on the TODO list aiming at improving the speed of the
> implementation will reduce the difference between cpp_int and the GMP
> equivalent even further.

...And get the number of digits higher, if we can find the right algorithms.

<snip>

> It was easy to use the cpp_dec_float or gmp_float classes in the context of
> complex numbers.

Excellent! Thank you. We are looking forward to extended complex as well.

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

> If boost wants to move into the realm of extended precision numbers, then
> yes.

Matthieu, you need to say yes or no. The reviewers decide and
it's Boolean.

<snip>

> Regards, Matthieu

Best regards, Chris.

[Matthieu Schaller]

>
> >> Do you think the library should be accepted as a Boost library?
>
> > If boost wants to move into the realm of extended precision numbers, then
> > yes.
>
> Matthieu, you need to say yes or no. The reviewers decide and
> it's Boolean.
>
>

Then, yes, it should be accepted.


Regards,

Matthieu

[John Maddock]

>> As a single example of this the gmp_int backend triggers a division by 0
>> signal when one tries to divide the integer by 0; the tom_int raises a
>> hardward signal with a division by 0; the cpp_int throws a
>> std::runtime_error with a division by 0. I would like to see some means
>> by which I could use any integer backend and know that a
>> std::runtime_error would be thrown by a division by 0.
>
> IMO, you should just avoid division by zero period.
> As far as C++ itself is concerned, division by 0
> is undefined behavior, so any code that wants to
> handle any numeric type including builtins can't
> assume anything about division by zero.

True, but.... I have some sympathy for Edwards POV, and we could be more
newbie friendly in that area.

My original thought was "don't second guess the backend" - in other words
let the backend just do whatever it does for each operation, and not try to
manhandle different backends towards the same behavior. However, on
reflection, division by zero is relatively cheap to test for compared to a
full division anyway, so we could check that particular case (or add an
extra template param to the backend I guess).

John.

[John Maddock]

> Here are my comments on the code so far.
> It's not very much, since I only got
> through a few files. I'll try to expand
> on this as I have time, but no promises.

Many thanks Steven.

> concepts/mp_number_architypes.hpp:
>
> * The file name is misspelled. It should
> be "archetypes."

Indeed, fixed in my local copy.

> * The #includes aren't quite right. The
> file does not use trunc, and does use
> fpclassify.
>
> * line 28: mpl::list requires #include <boost/mpl/list.hpp>
> * line 79: stringstream requires #include <sstream>

Fixed in my local copy.

> depricated:
>
> * Should be deprecated.

Will go away if the library is accepted anyway (just contains bits and bobs
not in the submission that I didn't want to delete just yet).

> detail/functions/constants.hpp:
>
> * line 12: typedef typename has_enough_bits<...> pred_type;
> If you're going to go through all this trouble to make
> sure that you have a type with enough bits, you shouldn't
> assume that unsigned has enough. IIRC, the standard
> only guarantees 16 bits for unsigned.

Nod. There's some new code that simplifies all that as well, simplified and
fixed in my local copy.

> * line 38: if(digits < 3640)
> Ugh. I'm assuming that digits means binary digits
> and 3640 = floor(log_2 10^1100)

Comments to that effect throughout.

> * line 51: I would appreciate a brief description
> of the formula. This code isn't very readable.

Nod. Added locally:

//
// We calculate log2 from using the formula:
//
// ln(2) = 3/4 SUM[n>=0] ((-1)^n * N!^2 / (2^n(2n+1)!))
//
// Numerator and denominator are calculated separately and then
// divided at the end, we also precalculate the terms up to n = 5
// since these fit in a 32-bit integer anyway.
//
// See Gourdon, X., and Sebah, P. The logarithmic constant: log 2, Jan.
2004.
// Also http://www.mpfr.org/algorithms.pdf.
//

Also spotted a missing optimisation in the arithmetic.

> * line 105: It looks like this is using the formula
> e \approx \sum_0^n{\frac{1}{i!}} = (\sum_0^n\frac{n!}{i!})/n!
> and induction using the rule:
> \sum_0^n\frac{n!}{i!} =
> (\sum_0^(n-1)\frac{(n-1)!}{i!}) * n + 1
>
> Please explain the math or point to somewhere
> that explains it.

Nod adding locally:

//
// Standard evaluation from the definition of e:
http://functions.wolfram.com/Constants/E/02/
//

> * line 147: ditto for pi

Nod. Adding locally:

//
// This algorithm is from:
// Schonhage, A., Grotefeld, A. F. W., and Vetter, E. Fast Algorithms: A
Multitape Turing
// Machine Implementation. BI Wissenschaftverlag, 1994.
// Also described in MPFR's algorithm guide:
http://www.mpfr.org/algorithms.pdf.
//
// Let:
// a[0] = A[0] = 1
// B[0] = 1/2
// D[0] = 1/4
// Then:
// S[k+1] = (A[k]+B[k]) / 4
// b[k] = sqrt(B[k])
// a[k+1] = a[k]^2
// B[k+1] = 2(A[k+1]-S[k+1])
// D[k+1] = D[k] - 2^k(A[k+1]-B[k+1])
// Stop when |A[k]-B[k]| <= 2^(k-p)
// and PI = B[k]/D[k]

Currently testing those changes before moving on to the other comments,

Thanks again, John.

[Jeff Flinn]

On 6/25/2012 4:10 AM, John Maddock wrote:
>>> As a single example of this the gmp_int backend triggers a division by 0
>>> signal when one tries to divide the integer by 0; the tom_int raises a
>>> hardward signal with a division by 0; the cpp_int throws a
>>> std::runtime_error with a division by 0. I would like to see some means
>>> by which I could use any integer backend and know that a
>>> std::runtime_error would be thrown by a division by 0.
>>
>> IMO, you should just avoid division by zero period.
>> As far as C++ itself is concerned, division by 0
>> is undefined behavior, so any code that wants to
>> handle any numeric type including builtins can't
>> assume anything about division by zero.
>
> True, but.... I have some sympathy for Edwards POV, and we could be more
> newbie friendly in that area.
>
> My original thought was "don't second guess the backend" - in other
> words let the backend just do whatever it does for each operation, and
> not try to manhandle different backends towards the same behavior.
> However, on reflection, division by zero is relatively cheap to test for
> compared to a full division anyway, so we could check that particular
> case (or add an extra template param to the backend I guess).

FYI, Several years ago, implementing a Runge Kutta integrator and
solver, we found that removing the explicit divide by zero check and
relying on the hardware signal to propagate as an exception gave a 40 -
60% performance improvement. Though, I'm not sure how that translates to
today's hardware/compilers and data types used.

Jeff

[John Maddock]

> FYI, Several years ago, implementing a Runge Kutta integrator and solver,
> we found that removing the explicit divide by zero check and relying on
> the hardware signal to propagate as an exception gave a 40 - 60%
> performance improvement. Though, I'm not sure how that translates to
> today's hardware/compilers and data types used.

Interesting - thanks!

I take it this was with native int's and not extended precision ones? I ask
because typical long division algorithms are rather expensive - as in O(N^2)
with a biggish constant as well.

That said, values that are small enough to fit in an int, but are actually
in an extended type "just in case" are an important use case, and as
supporting that case efficiently still needs more work, I guess I shouldn't
hamper that effort too much.... so I guess it's back to a compile time
switch....

John.

[Jeff Flinn]

On 6/25/2012 11:15 AM, John Maddock wrote:
>> FYI, Several years ago, implementing a Runge Kutta integrator and
>> solver, we found that removing the explicit divide by zero check and
>> relying on the hardware signal to propagate as an exception gave a 40
>> - 60% performance improvement. Though, I'm not sure how that
>> translates to today's hardware/compilers and data types used.
>
> Interesting - thanks!
>
> I take it this was with native int's and not extended precision ones? I

Yes, these were purely native types.

> ask because typical long division algorithms are rather expensive - as
> in O(N^2) with a biggish constant as well.
>
> That said, values that are small enough to fit in an int, but are
> actually in an extended type "just in case" are an important use case,
> and as supporting that case efficiently still needs more work, I guess I
> shouldn't hamper that effort too much.... so I guess it's back to a
> compile time switch....

In our case the incidence of divide by zero was a very small percentage
of the total iterations. The real cost of the if(...) was during all of
the non-zero iterations. As you say it could be negligible compared to
the other work being done.

Jeff

[John Maddock]

> * line 251: calc_pi(result, ...digits2<...>::value + 10);
> Any particular reason for +10? I'd prefer
> calc_pi to do whatever adjustments are
> needed to get the right number of digits.

It looks to be an "implementation artifact", I'm investigating removing it.

> detail/functions/pow.hpp:
>
> * line 18 ff: pow_imp
> I don't like the fact that this implementation
> creates log2(p) temporaries on the
> stack and multiplies them all together
> on the way out of the recursion. I'd
> prefer to only use a constant number
> of temporaries.
>
> * line 31:
> I don't see how the cases > 1 buy you anything.
> They don't change the number of calls to eval_multiply.
> (At least, it wouldn't if the algorithm were
> implemented correctly.)
> * line 47:
> ???: The work of this loop is repeated at every
> level of recursion. Please fix this function.
> It's a well-known algorithm. It shouldn't be
> hard to get it right.

Chris?

> * line 76: pow_imp(denom, t, -p, mpl::false_());
> This is broken for p == INT_MIN with twos-complement
> arithmetic.

Fixed locally.

> * line 185: "The ldexp function..."
> I think you mean the "exp" function.

Nod, fixed locally.

> * line 273: if(x.compare(ll) == 0)
> This is only legal if long long is in T::int_types.
> Better to use intmax_t which is guaranteed to be in
> T::int_types.

Nod. Fixed locally, there were a couple of other cases as well.

> * line 287: const bool b_scale = (xx.compare(float_type(1e-4)) > 0);
> Unless I'm missing something, this will always be
> true because xx > 1 > 1e-4 as a result of the
> test on line 240.

Yep. Fixed locally. My fault for "improving" Chris's original algorithm.

> * line 332: if(&arg == &result)
> This is totally unnecessary, since you already copy
> arg into a temporary using frexp on line 348.

Nod, fixed locally.

> * line 351: if(t.compare(fp_type(2) / fp_type(3)) <= 0)
> Is the rounding error evaluating 2/3 important?
> I've convinced myself that it works, but it
> took a little thought. Maybe add a comment to
> the effect that the exact value of the boundary
> doesn't matter as long as its about 2/3?

Not sure, Chris?

> * line 413: "The fabs function "
> s/fabs/log10/.

Fixed locally.

> * line 464: eval_convert_to(&an, a);
> This makes the assumption that conversion
> to long long is always okay even if the
> value is out of the range of long long. This
> assumption makes me nervous.

Needs a try{}catch{} at the very least. Fixed locally.

> * line 465: a.compare(an)
> compare(long long)

Yep.

> * line 474:
> eval_subtract(T, long long)
> Also, this value of da is never used. It
> gets overwritten on line 480 or 485.

Nod. Fixed locally.

> * line 493: (eval_get_sign(x) > 0)
> This expression should always be true at this point.

Nod. Fixed locally.

> * line 614: *p_cosh = x;
> cosh is an even function, so this is wrong when
> x = -\inf

Nod. Fixed locally.

> * line 625: T e_px, e_mx;
> These are only used in the generic implementation.
> It's better to restrict the scope of variables
> to the region that they're actually used in.

Nod. Fixed locally.

> * line 643: eval_divide(*p_sinh, ui_type(2));
> I notice that in other places you use ldexp for
> division by powers of 2. Which is better?

Good question, this is translated from Chris's original code which was
specific to his decimal floating point code where division by 2 is
(slightly) cheaper 'cos there's no simple ldexp. In the general case
though, ldexp is usually a better bet as it's often a simple bit-fiddle.
Changed locally.

> detail/mp_number_base.hpp:
>
> * line 423: (std::numeric_limits<T>::digits + 1) * 1000L
> This limits the number of bits to LONG_MAX/1000.
> I'd like there to be some way to know that as
> long as I use less than k bits, the library
> won't have any integer overflow problems. k
> can depend on INT_MAX, LONG_MAX, etc, but the
> implementation limits need to be clearly documented
> somewhere. I really hate the normal ad hoc
> situation where you just ignore the problem and
> hope that all the values are small enough to avoid
> overflow.

If there are really more digits than fit in a long then we'll likely have
other issues - such as not enough memory to store them all in - however,
I've added a static_assert and a note to this effect.

> Other notes:
>
> Backend Requirements:
>
> * Do we always require that all number types
> be assignable from intmax_t, uintmax_t,
> and long double? Why must an integer
> be assignable from long double?

Good question. It feels right to me that wherever possible all types should
have uniform requirements, but given that long double -> builtin int
conversions are always possible, why not long double -> extended int? In
other words it follows the idea that multiprecision types should as far as
possible behave like builtin types. The conversion should probably be
explicit in mp_number though (that's one on the TODO list).

> * The code seems to assume that it's okay to
> pass int as the exp parameter of ldexp.
> These requirements don't mandate that this
> will work.

That's a bug in the requirements, will fix.

Many thanks for the detailed comments, John.

[Christopher Kormanyos]

>> detail/functions/pow.hpp:
>>
>> * line 18 ff: pow_imp
>>  I don't like the fact that this implementation
>>  creates log2(p) temporaries on the
>>  stack and multiplies them all together
>>  on the way out of the recursion.  I'd
>>  prefer to only use a constant number
>>  of temporaries.
>>
>> * line 31:
>>  I don't see how the cases > 1 buy you anything.
>>  They don't change the number of calls to eval_multiply.
>>  (At least, it wouldn't if the algorithm were
>>  implemented correctly.)
>> * line 47:
>>  ???: The work of this loop is repeated at every
>>  level of recursion. Please fix this function.
>>  It's a well-known algorithm.  It shouldn't be
>>  hard to get it right.

> Chris?

Good suggestion. The function has been rewritten in the
sandbox using the S-and-X binary method, as described in
D. E. Knuth, "The Art of Computer Programming", Vol. 2,
Section 4.6.3.

Oh no, I put it in the sandbox. I should have fixed it locally,
as we are in a review. My bad. I am sorry if this is wrong.
John, a merge with your checked out copy will be needed.

<snip>

>> * line 351: if(t.compare(fp_type(2) / fp_type(3)) <= 0)
>>  Is the rounding error evaluating 2/3 important?
>>  I've convinced myself that it works, but it
>>  took a little thought.  Maybe add a comment to
>>  the effect that the exact value of the boundary
>>  doesn't matter as long as its about 2/3?

>Not sure, Chris?

I need to look at this function more closely. It needs more
comments through-and-through. I will perform some
boundary checking and add comments. I need a day or two
for this change request.

<snip>

> Many thanks for the detailed comments, John.

Thanks for the great suggestions, Chris.

[Edward Diener]

On 6/25/2012 4:10 AM, John Maddock wrote:

>>> As a single example of this the gmp_int backend triggers a division by 0
>>> signal when one tries to divide the integer by 0; the tom_int raises a
>>> hardward signal with a division by 0; the cpp_int throws a
>>> std::runtime_error with a division by 0. I would like to see some means
>>> by which I could use any integer backend and know that a
>>> std::runtime_error would be thrown by a division by 0.
>>
>> IMO, you should just avoid division by zero period.
>> As far as C++ itself is concerned, division by 0
>> is undefined behavior, so any code that wants to
>> handle any numeric type including builtins can't
>> assume anything about division by zero.
>
> True, but.... I have some sympathy for Edwards POV, and we could be more
> newbie friendly in that area.
>
> My original thought was "don't second guess the backend" - in other
> words let the backend just do whatever it does for each operation, and
> not try to manhandle different backends towards the same behavior.

The problem from the end-user's POV is that when different backends do
different things the end user has to be aware of the backend
implementation and take action accordingly. This complicates generic use
of your library somewhat for an end-user.

> However, on reflection, division by zero is relatively cheap to test for
> compared to a full division anyway, so we could check that particular
> case (or add an extra template param to the backend I guess).

I think choosing division by zero as an example was flawed as Steven
pointed out. My general point is that the library should strive to
provide compile-time alternatives which would regularize any differences
in backends as much as possible. Of course if this comes at the cost of
numeric interoperability and ease of use, I would be against any such
regularization. But I think that the individual cases should be
considered carefully.

[John Maddock]

>Oh no, I put it in the sandbox. I should have fixed it locally,
>as we are in a review. My bad. I am sorry if this is wrong.
>John, a merge with your checked out copy will be needed.

No problem, it merged OK, however it does fail some of the tests (incorrect
conceptual assumptions)... testing a fix now, John.

[Christopher Kormanyos]

>> Oh no, I put it in the sandbox. I should have fixed it locally,
>> as we are in a review. My bad. I am sorry if this is wrong.
>> John, a merge with your checked out copy will be needed.

> No problem, it merged OK, however it does fail some of the tests (incorrect
> conceptual assumptions)... testing a fix now,  John.

Oh, I see what you mean. The root of the problem runs deeper than
my trivial optimization of the pow() function. It's a bit beyond my
comprehension right now.

I re-wrote the assignment to one about ten different ways and the
disturbed tests fail on all re-writes. The solution must go deeper.

John, did your fix work?

Thanks, Chris.

[Christopher Kormanyos]

>>> Oh no, I put it in the sandbox. I should have fixed it locally,
>>> as we are in a review. My bad. I am sorry if this is wrong.
>>> John, a merge with your checked out copy will be needed.

>> No problem, it merged OK, however it does fail some of the tests (incorrect
>> conceptual assumptions)... testing a fix now,  John.

> I re-wrote the assignment to one about ten different ways and the

> disturbed tests fail on all re-writes. The solution must go deeper.
> John, did your fix work?
> Thanks, Chris.

Oh, I finally noticed that the my proposed pow_imp() function is
in the abstraction layer that can only call the "eval_"-style functions.
My assignment to the value of 1 would potentially need an "eval_set()"
to avoid ambiguous resolution.

The workaround below does pass the tests. In order to set the result
of the pow:imp() to one, however, I first multiply with zero to clear
the result, and subsequently add 1 to get 1---roundabout at best.

Do we need an eval_set() that is callable from the "detail" layer?

Thanks, Chris.

----------------- Bad WORKAROUND -----------------------------

namespace detail{

template<typename T, typename U>
inline void pow_imp(T& result, const T& t, const U& p, const mpl::false_&)
{
   // Compute the pure power of typename T
t^p.
   // Use the S-and-X binary method, as described in
   // D. E. Knuth, "The Art of Computer Programming", Vol. 2,
   // Section 4.6.3 . The resulting computational complexity
   // is order log2[abs(p)].

   U p2(p);

   // Determine if p is even or odd.
   const bool p_is_odd = (U(p2 % 2) != U(0));

   if(p_is_odd)
   {
      result = t;
   }
   else
   {
      eval_multiply(result, 0LL);
      eval_add(result, 1LL);
   }

   // The variable x stores the binary powers of t.
   T x(t);

   while(U(p2 /= 2) != U(0))
   {
      // Square x for each binary power.
     
eval_multiply(x, x);

      const bool has_binary_power = (U(p2 % U(2)) != U(0));

      if(has_binary_power)
      {
         // Multiply the result with each binary power contained in the exponent.
         eval_multiply(result, x);
      }
   }
}

...

[John Maddock]

> No problem, it merged OK, however it does fail some of the tests
> (incorrect
> conceptual assumptions)... testing a fix now, John.

>I re-wrote the assignment to one about ten different ways and the
>disturbed tests fail on all re-writes. The solution must go deeper.
>
>John, did your fix work?

Yes, I haven't committed yet though as there are other changes mixed in with
it that I haven't finished with yet, John.

[John Maddock]

>Do we need an eval_set() that is callable from the "detail" layer?

Nope, there's already an operator= for all the backends, but the argument
must be a type listed in one of the backends typelists, so the fix is:

// Find the type as wide as U that T is assignable from:
typedef typename boost::multiprecision::detail::canonical<U, T>::type
int_type;

// This will store the result.
if(U(p % U(2)) != U(0))
{
result = t;
}
else
result = int_type(1);

John.

[Christopher Kormanyos]

>> Do we need an eval_set() that is callable from the "detail" layer?


> Nope, there's already an operator= for all the backends, but the
> argument must be a type listed in one of the backends typelists,
> so the fix is:

>  // Find the type as wide as U that T is assignable from:
>  typedef typename boost::multiprecision::detail::canonical<U, T>::type int_type;
>
>  // This will store the result.
>  if(U(p % U(2)) != U(0))
>  {
>    result = t;
>  }
>  else
>    result = int_type(1);

> John.

Thanks yet again.

I got your update from the sandbox and tested it. The tests in my
checkout of the trunk (without specfun) are OK with VS2010.

It's amazing how quickly and reliably you work!

Best regards, Chris.

[Dave Abrahams]

on Mon Jun 25 2012, John Maddock <boost.regex-AT-virgin.net> wrote:

>>> As a single example of this the gmp_int backend triggers a division by 0
>>> signal when one tries to divide the integer by 0; the tom_int raises a
>>> hardward signal with a division by 0; the cpp_int throws a
>>> std::runtime_error with a division by 0. I would like to see some means
>>> by which I could use any integer backend and know that a
>>> std::runtime_error would be thrown by a division by 0.
>>
>> IMO, you should just avoid division by zero period.
>> As far as C++ itself is concerned, division by 0
>> is undefined behavior, so any code that wants to
>> handle any numeric type including builtins can't
>> assume anything about division by zero.
>
> True, but.... I have some sympathy for Edwards POV, and we could be
> more newbie friendly in that area.
>
> My original thought was "don't second guess the backend" - in other
> words let the backend just do whatever it does for each operation, and
> not try to manhandle different backends towards the same behavior.
> However, on reflection, division by zero is relatively cheap to test
> for compared to a full division anyway, so we could check that
> particular case (or add an extra template param to the backend I
> guess).

Not only that, but not everybody is writing platform-independent code,
and this goes double for people involved in HPC. A long-running
calculation that goes down because of divide-by-zero can be extremely
expensive. A nod should be given toward ways of handling these issues.

--
Dave Abrahams
BoostPro Computing
http://www.boostpro.com

[Mario Mulansky]

Hi

On Friday, June 08, 2012 04:28:09 PM Jeffrey Lee Hellrung, Jr. wrote:
> Okay, it's that time. Apologies for not sending a reminder, but there had
> been a lot of pre-review comments over the last week or so, so I think this
> has been on at least some people's radar. The original review announcement
> is given below.

I hope it is not too late to give a short brief review of my experience with
the Multiprecision library. First a general comment: this review is from a
"naive user" perspective who just wants to use multiprecision and is happy
finding a C++ library for that.

My test case is to use floating point multiprecision types as the basis for
our odeint library (www.odeint.com). As odeint is fully templatized I could
just plug in, say, the cpp_dec_float_50 and the basic routines worked out of
the box. The only problem I had was that there seems to be no support for min
/ max functions for expressions of multiprecision types. So can not write

x = max( a+b , a*b )

when x,a,b are some mp types, however

a1 = a+b
a2 = a*b
x = max( a1 , a2 )

works and I used it as a workaround, however I would like to see full support
of min/max in Multiprecision (maybe I just missed something?)

I can not say anything on the design or implementation as I did not look into
it. May only remark is that the design is such that mp types work nicely with
large numerical libraries and the implementation seems to be robust.

Some basic performance test of cpp_dec_float_50 vs mpf_float_50 gave me a
factor of about 1.8 in favor of mpf, as expected from what I read in the docs.

As for the documentation I didn't go beyond the introduction, because I didn't
need too. This is exactly how it should be, if you just want to use a library
the Introduction/Tutorial should be all you have to read.

To conclude this very brief review, I would be happy to see this library
becoming a part of boost.

Best, Mario

>
> On Tue, May 29, 2012 at 2:08 PM, Jeffrey Lee Hellrung, Jr. <
>
> jeffrey....@gmail.com> wrote:
> > 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.
> >
> > From the Introduction:
> >
> > --------
> > "The Multiprecision Library provides *User-defined* integer, rational and
> > floating-point C++ types which try to emulate as closely as practicable
> > the C++ built-in types, but provide for more range and precision.
> > Depending upon the number type, precision may be arbitrarily large
> > (limited only by available memory), fixed at compile time values, for
> > example 50 decimal digits, or a variable controlled at run-time by
> > member functions. The types are expression-template-enabled for better
> > performance than naive user-defined types."
> > --------
> >
> > And from the original formal review request from John:
> >
> > --------
> > Features:
> >
> > * Expression template enabled front end.
> > * Support for Integer, Rational and Floating Point types.
> >
> > Supported Integer backends:
> >
> > * GMP.
> > * Libtommath.
> > * cpp_int.
> >
> > cpp_int is an all C++ Boost licensed backend, supports both arbitrary
> > precision types (with Allocator support), and signed and unsigned fixed
> > precision types (with no memory allocation).
> >
> > There are also some integer specific functions - for Miller Rabin
> > testing, bit fiddling, random numbers. Plus interoperability with
> > Boost.Rational (though that loses the expression template frontend).
> >
> > Supported Rational Backends:
> >
> > * GMP
> > * libtommath
> > * cpp_int (as above)
> >
> > Supported Floating point backends:
> >
> > * GMP
> > * MPFR
> > * cpp_dec_float
> >
> > cpp_dec_float is an all C++ Boost licensed type, adapted from Christopher
> > Kormanyos' e_float code (published in TOMS last year).
> >
> > All the floating point types, have full std lib support (cos sin exp, pow
> > etc), as well as full interoperability with Boost.Math.
> >
> > There's nothing in principal to prevent extension to complex numbers and
> > interval arithmetic types (plus any other number types I've forgotten!),
> > but I've run out of energy for now ;-)
> >
> > Code is in the sandbox under /big_number/.
> >
> > Docs can be viewed online here:
> > http://svn.boost.org/svn/boost/sandbox/big_number/libs/multiprecision/doc
> > /html/index.html --------

>
> Any review discussion should take place on the developers' list (
> bo...@lists.boost.org), and anyone may submit a formal review, either
> publicly to the entire list or privately to just myself.
>
> As usual, please consider the following questions in your formal review:
>

> What is your evaluation of the design?

> What is your evaluation of the implementation?

> What is your evaluation of the documentation?

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

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

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

> Are you knowledgeable about the problem domain?
>

> And, most importantly, please explicitly answer the following question:
>

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

> Lastly, please consider that John and Christopher have compiled a TODO list
> [1] based on pre-review comments. Feel free to comment on the priority and
> necessity of such TODO items, and whether any might be show-stoppers or
> warrant conditional acceptance of the library.
>

> Thanks in advance; looking forward to the discussion!
>
> - Jeff (& John & Christopher)
>
> [1]
> http://svn.boost.org/svn/boost/sandbox/big_number/libs/multiprecision/doc/h
> tml/boost_multiprecision/map/todo.html

[Marc Glisse]

On Thu, 28 Jun 2012, Mario Mulansky wrote:

> My test case is to use floating point multiprecision types as the basis for
> our odeint library (www.odeint.com). As odeint is fully templatized I could
> just plug in, say, the cpp_dec_float_50 and the basic routines worked out of
> the box. The only problem I had was that there seems to be no support for min
> / max functions for expressions of multiprecision types. So can not write
>
> x = max( a+b , a*b )

x = max<cpp_dec_float_50>( a+b , a*b )
maybe?

(assuming they don't overload max for this library)

--
Marc Glisse

[Christopher Kormanyos]

Thank you for taking the time to try the library, Mario.

> I hope it is not too late to give a short brief review of my experience with
> the Multiprecision library. First a general comment: this review is from a
> "naive user" perspective who just wants to use multiprecision and is happy
> finding a C++ library for that.

Users just wanting big number types are *the* target group.
In fact, I consider myself to be a naive user.

> My test case is to use floating point multiprecision types as the basis for
> our odeint library (www.odeint.com). As odeint is fully templatized I could
> just plug in, say, the cpp_dec_float_50 and the basic routines worked out of
> the box. The only problem I had was that there seems to be no support for min
> / max functions for expressions of multiprecision types. So can not write

> x = max( a+b , a*b )

It works when the template parameter of std::max() is explicitly provided.
The code below, for example, successfully compiles.

boost::multiprecision::cpp_dec_float_100 a(boost::multiprecision::cpp_dec_float_100(1) / 3);
boost::multiprecision::cpp_dec_float_100 b(boost::multiprecision::cpp_dec_float_100(1) / 7);
boost::multiprecision::cpp_dec_float_100 c = std::max<boost::multiprecision::cpp_dec_float_100>(a + b, a * b);

It does seem strange that the compiler needs an explicit template
parameter for dis-ambiguity regarding the results of binary arithmetic.

I don't have enough karma to know.
John, what's your opinion?

> when x,a,b are some mp types, however

> a1 = a+b
> a2 = a*b
> x = max( a1 , a2 )

> works and I used it as a workaround, however I would like to see full support
> of min/max in Multiprecision (maybe I just missed something?)

<snip>

Best, Mario

<snip>

Thanks again, Mario.
Best regards, Chris.

[Simonson, Lucanus J]

Mario wrote:
>> x = max( a+b , a*b )

Christopher wrote:
>It works when the template parameter of std::max() is explicitly provided.
>The code below, for example, successfully compiles.

>boost::multiprecision::cpp_dec_float_100 c = std::max<boost::multiprecision::cpp_dec_float_100>(a + b, a * b);
>It does seem strange that the compiler needs an explicit template parameter for dis-ambiguity regarding the results of binary arithmetic.

The result of the operator function call is an expression template (I'm guessing he had expression templates enabled). You are forcing an implicit cast to temporaries of the number type by specifying the max template parameter. a+b has a different expression template type than a*b. Since max takes only one template parameter you can't specify two different types as arguments and expect it to be able to deduce.

template <class T> const T& max(const T& a, const T& b);

Presumably disabling expression templates would have worked for Mario because a+b and a*b would result in temporaries both of type cpp_dec_float_100 rather than temporaries of different expression template types.

Regards,
Luke

[Edward Diener]

I did not want to focus on just divide by zero. I wanted to suggest that
any anomalies between backends using the same general type be normalized
as much as it is possible to do effectively in order to promote generic
use of the backend.

As a further example the doc for the cpp_int type says:

"Construction from a string that contains invalid non-numeric characters
results in a std::runtime_error being thrown. "

I do not see any mention if this is the case for gmp_int and/or tom_int.
If it is not the same then a programmer constructing an mp_number using
a cpp_int_backend<> which passes a string potentially containing an
invalid non_numeric character must handled the mp_number constructor
differently than when constructing using a gmp_int or a tom_int backend.
But ideally one wants to choose any effective backend and then have all
functionality be consistently the same and hopefully equally as correct
as possible. My point is simply that I believe the library should
normalize ( or "regularize" ) any differences as much as possible as
long as this does not affect the accuracy ( and perhaps speed ) of the
library. Of course there are always tradeoffs but a generic library
should always work to minimize differences wherever possible.

[Mario Mulansky]

On Thu, 2012-06-28 at 19:38 +0200, Marc Glisse wrote:
> On Thu, 28 Jun 2012, Mario Mulansky wrote:
>
> > My test case is to use floating point multiprecision types as the basis for
> > our odeint library (www.odeint.com). As odeint is fully templatized I could
> > just plug in, say, the cpp_dec_float_50 and the basic routines worked out of
> > the box. The only problem I had was that there seems to be no support for min
> > / max functions for expressions of multiprecision types. So can not write
> >
> > x = max( a+b , a*b )
>
> x = max<cpp_dec_float_50>( a+b , a*b )
> maybe?
>
> (assuming they don't overload max for this library)

I think this would work, but I don't want to do that because then any
user defined non-templatized function would not be found (see example
below). I think the better way is an explicit cast of the arguments:

max( static_cast< mp_type >( a+b ) , static_cast< mp_type >( a*b ) );

as also written in the MP introduction as I just found out.

Example, where the user defined function is not used because it isn't
templatized:

#include <iostream>

struct my_type
{ };


bool operator <( const my_type x , const my_type y )
{ return true; } // for std::max to compile...

bool max( const my_type x , const my_type y )
{
std::cout << "test" << std::endl;
};

int main()
{
my_type a , b;

using std::max;

max( a , b ); // calls user defined max

max<my_type>( a , b ); // calls std::max, not good

// user defined max, I think that's the right way:
max( static_cast<my_type>(a) , static_cast<my_type>(b) );

[John Maddock]

> I do not see any mention if this is the case for gmp_int and/or tom_int.
> If it is not the same then a programmer constructing an mp_number using a
> cpp_int_backend<> which passes a string potentially containing an invalid
> non_numeric character must handled the mp_number constructor differently
> than when constructing using a gmp_int or a tom_int backend. But ideally
> one wants to choose any effective backend and then have all functionality
> be consistently the same and hopefully equally as correct as possible. My
> point is simply that I believe the library should normalize ( or
> "regularize" ) any differences as much as possible as long as this does
> not affect the accuracy ( and perhaps speed ) of the library. Of course
> there are always tradeoffs but a generic library should always work to
> minimize differences wherever possible.

Nod. I will certainly try and normalize behavior where practical, and those
are good examples to look at further,

Thanks, John.

[John Maddock]

> The only problem I had was that there seems to be no support for min
> / max functions for expressions of multiprecision types. So can not write
>
> x = max( a+b , a*b )

There is a mention of this in the introduction about 2/3 of the way down
http://svn.boost.org/svn/boost/sandbox/big_number/libs/multiprecision/doc/html/boost_multiprecision/intro.html

The problem is this:

The result of a+b is an expression template, the result of a*b is different
expression template type, so:

1) Template argument deduction for std::max fails.
2) Even if template argument deduction had succeeded, you would be passing
the "wrong type" to the function - an expression template and not a number,
so for example given:

foo(a+b)

template arg deduction would succeed for foo (assuming foo is a template
function), but you'd likely get inscrutable errors inside the body of foo as
a result of passing something that's not actually a number to it.

So you either have to:

* Explicitly cast the arguments to the underlying number type when calling a
template function. Or,
* Call an explicit template instance by passing the function the template
args inside <>.

Note that none of the above applies when calling non-template functions as
all the usual conversions apply, I've also fixed up the multiprecision and
Math lib's so they interoperate without having to worry about any of this.
I'd forgotten all about std::min/max, but I'd add overloads for those too
(and any other std lib functions I spot).

Cheers, John.

[Steven Watanabe]

AMDG

On 06/25/2012 10:44 AM, John Maddock wrote:
>>
>> * line 423: (std::numeric_limits<T>::digits + 1) * 1000L
>> This limits the number of bits to LONG_MAX/1000.
>> I'd like there to be some way to know that as
>> long as I use less than k bits, the library
>> won't have any integer overflow problems. k
>> can depend on INT_MAX, LONG_MAX, etc, but the
>> implementation limits need to be clearly documented
>> somewhere. I really hate the normal ad hoc
>> situation where you just ignore the problem and
>> hope that all the values are small enough to avoid
>> overflow.
>
> If there are really more digits than fit in a long then we'll likely
> have other issues - such as not enough memory to store them all in -
> however, I've added a static_assert and a note to this effect.
>

Taking into account the fact that
numeric_limits::digits is in bits
and the extra factor of 1000, it
only takes 500 KB to run into a
problem when long is 32 bits. This
is small enough that running out
of memory is not necessarily an issue.

In Christ,
Steven Watanabe

[Sergey Mitsyn]

Hello,

Not quite a review as it's way too late :), but some comments on
probably interesting experience I've got while trying out multiprecision
from sandbox together with Boost.UBLAS . Both libraries are generic in
the way that one can be used instead of builtin float and double, and
the other should support such types.

I've found out that they don't play well together, probably due to the
fact that UBLAS uses its own expression templates. A simple example of
code that fails to compile is below:

------------------

#include <boost/numeric/ublas/vector.hpp>
#include <boost/multiprecision/cpp_dec_float.hpp>

typedef boost::multiprecision::mp_number<
boost::multiprecision::cpp_dec_float<50> , true > float_type;

int main(int argc, char* argv[])
{
boost::numeric::ublas::c_vector<float_type, 3> v1,v2;
inner_prod( v1, v2 );
}

-----------------

Fails with MSVC 9 and a little old clang from thunk (--version prints
clang version 3.2 (trunk 156987)).

The easy fix is to turn off expression templates inside mp_number.

It also fails even with expression templates being turned off whenever
it tries to compare mp_number with a convertible type:

-------------------------

#include <boost/multiprecision/cpp_dec_float.hpp>

typedef boost::multiprecision::mp_number<
boost::multiprecision::cpp_dec_float<50> , false > float_type;


struct A
{
operator float_type() const
{
return (float_type)1.0f;
}
};

int main(int argc, char* argv[])
{
float_type f = 123.0f;
A a;

bool r = (a < f);
}

-------------------------

Such constructions appear sometimes within UBLAS (e.g., lu_factorize
with NDEBUG undefined). The fix is probably non-trivial. I've
successfully overcome it by modifying is_valid_comparison_imp:

------------------------

template <class Exp1, class Exp2>
struct is_valid_comparison_imp
{
<.......skipped.......>
mpl::and_<
is2,
mpl::or_<
is1,
is_arithmetic<Exp1>
>
>,
+ mpl::and_<
+ is_convertible<Exp1,Exp2>,
+ is2
+ >,
+ mpl::and_<
+ is_convertible<Exp2,Exp1>,
+ is1
+ >
>::type type;
};

----------------------------

Though I didn't run the tests - it might break something.

Also running the tests for UBLAS with mp_number put in place of
floats/doubles would be interesting.

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

Whenever issues with UBLAS interoperability are fixed (or some
reasonable decision is made).

--
------------
Sergey Mitsyn.

[John Maddock]

> Not quite a review as it's way too late :), but some comments on probably
> interesting experience I've got while trying out multiprecision from
> sandbox together with Boost.UBLAS . Both libraries are generic in the way
> that one can be used instead of builtin float and double, and the other
> should support such types.
>
> I've found out that they don't play well together, probably due to the
> fact that UBLAS uses its own expression templates. A simple example of
> code that fails to compile is below:
>
> ------------------
>
> #include <boost/numeric/ublas/vector.hpp>
> #include <boost/multiprecision/cpp_dec_float.hpp>
>
> typedef boost::multiprecision::mp_number<
> boost::multiprecision::cpp_dec_float<50> , true > float_type;
>
> int main(int argc, char* argv[])
> {
> boost::numeric::ublas::c_vector<float_type, 3> v1,v2;
> inner_prod( v1, v2 );
> }

I'm not surprised, third party template libraries would have to be
"expression template safe" to be used unchanged with mp_number, a typical
example of what fails is:

foo(a+b)

where foo is a template function. This can only be fixed inside uBlas
unfortunately.

> Fails with MSVC 9 and a little old clang from thunk (--version prints
> clang version 3.2 (trunk 156987)).
>
> The easy fix is to turn off expression templates inside mp_number.
>
> It also fails even with expression templates being turned off whenever it
> tries to compare mp_number with a convertible type:
>
> -------------------------
>
> #include <boost/multiprecision/cpp_dec_float.hpp>
>
> typedef boost::multiprecision::mp_number<
> boost::multiprecision::cpp_dec_float<50> , false > float_type;
>
>
> struct A
> {
> operator float_type() const
> {
> return (float_type)1.0f;
> }
> };
>
> int main(int argc, char* argv[])
> {
> float_type f = 123.0f;
> A a;
>
> bool r = (a < f);
> }
>
> -------------------------
>
> Such constructions appear sometimes within UBLAS (e.g., lu_factorize with
> NDEBUG undefined). The fix is probably non-trivial. I've successfully
> overcome it by modifying is_valid_comparison_imp:

Sigh... that's just nasty. I probably need to rewrite the comparison code
anyway for better efficiency, I'll try and fix this along the way.

> Also running the tests for UBLAS with mp_number put in place of
> floats/doubles would be interesting.

Nod, will try.

>> Do you think the library should be accepted as a Boost library?
>>
>
> Whenever issues with UBLAS interoperability are fixed (or some reasonable
> decision is made).

Many thanks for the feedback,

John.

[Jeffrey Lee Hellrung, Jr.]

On Sun, Jul 29, 2012 at 9:13 AM, Sergey Mitsyn <s...@jinr.ru> wrote:

> Hello,
>
> Not quite a review as it's way too late :), but some comments on probably
> interesting experience I've got while trying out multiprecision from
> sandbox together with Boost.UBLAS . Both libraries are generic in the way
> that one can be used instead of builtin float and double, and the other
> should support such types.
>

[...]

Thanks Sergey for your observations. I've added them to my partial list of
notes. (I'll finish up aggregating the discussion and post the review
results within a couple weeks, I hope.)

- Jeff

[John Maddock]

> Also running the tests for UBLAS with mp_number put in place of
> floats/doubles would be interesting.

I've added modified versions of uBlas tests 1-7 to the multiprecision lib,
most of the tests pass, but there are two outstanding issues:

* Test1: I had to disable a couple of tests because they trigger debug
assertons inside the STL. I get the same errors when testing with type
double, so I conclude it's a uBlas issue, not sure why they aren't triggered
in the regular regression tests though...
* Test3: I can't get this to compile, and the error are so inscrutable I
don't know what the issue is, I'd need some help from a uBlas expert to
figure those out.

Note that the above is with expression templates turned off in
multiprecision. Getting uBlas to work with an expression template enabled
number type would require extensive (but not difficult) modification of
uBlas. If anyone wants to take that on I can offer advice.

Cheers, John.

[John Maddock]

>> Also running the tests for UBLAS with mp_number put in place of
>> floats/doubles would be interesting.
>
> I've added modified versions of uBlas tests 1-7 to the multiprecision lib,
> most of the tests pass, but there are two outstanding issues:
>
> * Test1: I had to disable a couple of tests because they trigger debug
> assertons inside the STL. I get the same errors when testing with type
> double, so I conclude it's a uBlas issue, not sure why they aren't
> triggered in the regular regression tests though...
> * Test3: I can't get this to compile, and the error are so inscrutable I
> don't know what the issue is, I'd need some help from a uBlas expert to
> figure those out.

Hold on that was a mistake, test1 does pass, tests 3 and 7 don't compile for
reasons I don't understand. I'd need some help from a uBlas expert to
figure those out.

[Sergey Mitsyn]

On 07.08.2012 15:51, John Maddock wrote:
>>> Also running the tests for UBLAS with mp_number put in place of
>>> floats/doubles would be interesting.
>>
>> I've added modified versions of uBlas tests 1-7 to the multiprecision
>> lib, most of the tests pass, but there are two outstanding issues:
>>
>> * Test1: I had to disable a couple of tests because they trigger debug
>> assertons inside the STL. I get the same errors when testing with
>> type double, so I conclude it's a uBlas issue, not sure why they
>> aren't triggered in the regular regression tests though...
>> * Test3: I can't get this to compile, and the error are so inscrutable
>> I don't know what the issue is, I'd need some help from a uBlas expert
>> to figure those out.
>
> Hold on that was a mistake, test1 does pass, tests 3 and 7 don't compile
> for reasons I don't understand. I'd need some help from a uBlas expert
> to figure those out.

Not an expert, but looks like tests 3 and 7 are failing not only for
multiprecision:

http://www.boost.org/development/tests/release/developer/numeric-ublas.html

Test 7 is removed completely, and has a notion (inside test7.cpp) of
dependency on open bug #2473 of Boost.Interval . Probably it shouldn't
be considered at all.

Test 3 fails for some compilers, you may want to check if you use one of
them.

>
> John.
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>

--
------------
Sergey Mitsyn.