Re: [boost] [Review] accumulators

From: Eric Niebler (eric_at_[hidden])
Date: 2007-02-07 16:05:41

Bernhard Mäder wrote:
> Hello all
> 
> Hope I'm not too late with a small review of the accumulators library:
> 
>  • What is your evaluation of the design?
> 
>  I very much like the user interface of the library, it's pretty clean
> and concise; writing something like
> 
>  accumulator_set<double, stats<tag::median(with_p_square_quantile) > > acc;
> 
>  is as expressive as it gets. This is just cool.

Thanks!

>  On the other side of the library, when defining new features, things
> are a bit more complicated. I experienced some ugly compile-time
> troubles when trying to implement a group of features that depended on
> each other. Writing that code took considerable more time than writing
> the same functionality without the framework. I'm quite willing to
> take this effort, though, for all the benefits of the accumulators framework.

If you would be willing to share your experiences, I could amend the 
docs so that others can avoid stepping in the same potholes.

>  Then, I can't really get warm with the automatic feature resolution.
> On the one hand, it's a great feature for reducing accumulator set
> definition code. On the other hand, the programmer has to know which
> features get pulled in anyway (you'll need to include those header
> files, right?).

No, you shouldn't. If one accumulator depends on another, it should 
include the header for you.

  Furthermore, it seems a bit dangerous to have such
> automatisms in a field, where you should always know what you're doing
> anyway (which is again exemplified by the discussion of the various
> implementations of the mean and variance features on this list).

I disagree. Some statistics share intermediate results with other 
statistics. Those intermediate results may not be interesting -- may not 
even be a statistic! -- and users shouldn't have to specify them when 
constructing an accumulator set.

  Then
> there is BOOST_ACCUMULATORS_MAX_FEATURES, which has to take into
> account even those features that are pulled in automatically. This is
> a bit counter intuitive and caused some troubles on my side.

You're right about that. I plan to change the code to use a data 
structure that does not have this artificial limit. Look for 
BOOST_ACCUMULATORS_MAX_FEATURES to go away.

> 
>  I'd wish that accumulator_set provided an operator<< to print it to
> console or files. We're often using accumulator sets from an
> interactive python console, which would further benefit from a
> pretty-printed output. 

What should it print?

Equally, they should be made serializable.

Some accumulators need named construction parameters, and it's entirely 
up to each accumulator. Serialization support would be a non-trivial 
extension, I think.

The
> formerly discussed combine() feature could also come in very handy for
> some of our applications.
> 
>  Overall, I think the library's complexity is as high as I'm willing
> to go for every day's business. It's really nothing for C++ beginners
> to start with, as it requires some intermediate level "boost"
> thinking.
> 
>  • What is your evaluation of the implementation?
> 
>  The framework code is inspiring, as most of Eric's work. The stats
> are very well separated and pretty easy to read.
> 
>  • What is your evaluation of the documentation?
> 
>  A bit short at times. Some tools lack detailed description
> (depends_on, feature_of etc.), the external<> mechanics are not
> clearly described. The overall structure of the user's guide is very
> good, though.

Yes, a user-oriented description of depends_on, feature_of and 
as_feature would be very good.

>  • What is your evaluation of the potential usefulness of the library?
> 
>  Well, we're already using it in production code, so I guess it's very
> real. There are many other application fields where I could imagine
> using it (signal processing, imaging, embedded systems with limited
> memory etc.)
> 
>  • Did you try to use the library? With what compiler? Did you have
> any problems?
> 
>  We're using the library on debian sarge with GCC3.4. Apart for some
> long and obscure compiler outputs on errors (and that's hardly boost
> accumulator's fault), all went fine.

I consider long compiler errors for simple user errors to be a bug. If 
you tell me what you did to trigger the long errors, I can see about 
putting a strategically placed compile-time assert to give a better error.

> 
>  • How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
> 
>  I've been using the library as end-user for quite some time now. I
> did not study all of the source code nor all of the design decisions,
> but come across many aspects of the library during normal usage.
> 
>  • Are you knowledgeable about the problem domain?
> 
>  I do know our requirements for statistics on current project, and see
> where they cause problems, But I'm not really a statistics expert.
> 
>  • Do you think the library should be accepted as a Boost library?
> 
>  Of course, it's a great enhancement to boost.

Thanks, Bernhard.

-- 
Eric Niebler
Boost Consulting
www.boost-consulting.com