[boost] Accumulators library documentation review

From: Hans Meine (meine_at_[hidden])
Date: 2007-02-07 04:48:44

Hi Eric, John,

I wanted to send a full review for the library, but it looks as if I need a 
boost CVS checkout for that (for boost/typeof/typeof.hpp), and I am very 
short on time right now.

So I at least send what I already have: Lots of comments on the documentation:

Comments on the Documentation 
=============================

(Disclaimer: In some of the following, I purposely pretended to be
dumber than I am, in order to show where the explanations could use
some improvements. ;-) )

In the "Passing Optional Parameters", I read about tail and
tail_variate.  Not being an expert in statistics, I had problems
understanding the example, so I tried to find out what tail_variate<>
does.  Unfortunately, I could only find out that it takes three
template parameters (VariateType, VariateTag, LeftRight), but I could
not even see what they mean.  LeftRight is passed to tail<>, and then
to tail_cache_size_named_arg<>, but I still do not see what the
left/right is for.  -> Update: I found the answers in the Statistical
Accumulators Library's documentation.  It would be much better AFAICS
if the links of tail<> and tail_variate<> would point to that
appropriate documentation.

Having said that, I think it could be useful to include links to
external sites explaining some of the statistics terms, if you know
such.  I bet there are more people like me, who have only little more
than basic knowledge in statistics but who are interested in the more
complicated features once they start using your library.

A comment in a code example reads:
> All accumulators should inherit from accumulator_base.
Add a small note somewhere why this is necessary / what is inherited?
Also does "should" mean that this is mandatory?  (Update: OK, now I
saw the note later down in the docs about the default operator().
Still I don't know if that's all.)

Then you wrote:
> Although not necessary, it can be a good idea to put your accumulator
> implementations in the boost::accumulators::impl namespace. This
> namespace pulls in any operators defined in the
> boost::numeric::operators namespace with a using directive. The
> Numeric Operators Sub-Library defines some additional overloads that
> will make your accumulators work with all sorts of data types.
It does not look clean to me to put my own stuff in someone else's
namespaces; is there a problem with just "using namespace
numeric::operators;"?  (OK, which? ;-) )

Documentation of the Accumulator Concept: Maybe link back to "Optional
Accumulator Member Functions" for post_construct and on_drop?  (As it
stands, one can understand the concept definition only after reading
much of the documentation before, although one might want to quickly
check the concepts - possibly when coming from the first reference to
the concepts in the docs, which is before the explanations.)

In the Feature Concept, I cannot understand the explanation of
F::is_weight_accumulator - it seems to me as if a second occurence of
S (template parameter?) is missing somewhere?

Ah, now I see:
> The weight accumulators are made external if the weight type is
> specified using the external<> template.
Still, that part of the API is missing an example, isn't it?
I have difficulties understanding when and how to use external<>.

At the end, just above the TODO, there are two items:
> * Mapping multiple impls to the same feature with feature_of 
> * Creating aliases for features with as_feature
I guess these are also placeholders for future doc snippets?

The Statistical Accumulators Library
====================================

I was very glad to find formulas and article references for
non-trivial accumulators like the p^2 median estimators.  How about
sth. like that for more of the accumulators (e.g. "kurtosis", or even
for mean and variants like immediate_mean)?

I also wondered why you always write
> For implementation details, see foo_impl.
and "hide" the formulas there?

I think the statistical accumulators reference docs can be greatly improved 
with a better structuring.

Also, I very much dislike the look in my browser (Konqueror), but I guess it 
may be the fault of some "standard boost CSS".
Looking into it right now, it seems that since "The Statistical Accumulators 
Library" is a subsection of the user's guide, all sections inside it become 
either <h3> or <h4> and thus nearly-indistinguishably small (hardly larger 
font sizes than the usual text).  That should be probably discussed in a 
separate thread to get some attention by the responsible...

Greetings,
  Hans