Boost logo

Boost :

Subject: Re: [boost] [review] Multiprecision review (June 8th - 17th, 2012)
From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2012-06-17 23:55:42


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


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk