Boost logo

Boost :

From: Eric Niebler (eric_at_[hidden])
Date: 2007-01-30 12:47:43

Hi, Steven,

Steven Watanabe wrote:

Ad Majorem Dei Gloriam?

> user_s_guide.html#id450311 line 363
> accumulators.qbk line 189
> for some reason external<> is not a link in the html

Ah, the link is broken. Thanks.

> accumulators.qbk lines 35-37
> [def _mpl_ [@../../libs/mpl MPL]]
> [def _mpl_lambda_expression_
> [@../../libs/mpl/doc/refmanual/lambda-expression.html MPL Lambda
> Expression]]
> [def _parameters_ [@../../libs/parameters Boost.Parameters]]
> these links are bad

Thanks. I think I was overly-optimistic when I wrote that. The links
would work if Accumulators were in it's rightful (a-hem) place in the
Boost tree. ;-)

> post_construct()
> Unless for some obscure reason that I am missing it is just
> too difficult, I would strongly prefer to make sure the constructors
> are called in the correct order.
> The whole dance with fusion::vector is unnecessary.
> It would be simpler to use
> struct accumulator_tuple_base {
> template<class Args>
> accumulator_tuple(const Args&) {}
> }
> template<class T, class Base>
> struct accumulator_tuple : T, Base {
> template<class Args>
> accumulator_tuple(const Args& args) : T(args), Base(args) {}
> };
> Since every accumulator is put before all the
> accumulators that depend on it in make_accumulator_tuple
> you can write
> typedef
> typename mpl::reverse_fold<
> sorted_accumulator_vector
> , accumulator_tuple_base
> , accumulator_tuple<mpl::_2, mpl::_1>
> >::type
> type;
> Then every accumulator is constructed after any
> others that it depends on. As a bonus the only
> limit to the number of accumulators is whatever
> the compiler can handle.

This would be a valid design choice, but it would involve reinventing a
lot of Fusion. I use Fusion both to generate the accumulator set, to
process the accumulators, and to find accumulators that match Fusion
predicates. Code reuse is good.

Regarding post_process(), I decided at the time, after discussing this
with Joel de Guzman (of Fusion) and Dave Abrahams, that relying on the
construction order of accumulators within a set would be a mistake. It
would rule out size optimizations like compressed_pair, where empty
accumulators are optimized away via the Empty Base Optimization. Fusion
doesn't do this optimization today, but it may in the future.

> What happens if you freeze a feature that another feature depends on?
> #include <iostream>
> #include <boost/accumulators/accumulators.hpp>
> #include <boost/accumulators/statistics/mean.hpp>
> #include <boost/accumulators/statistics/sum.hpp>
> using namespace boost::accumulators;
> int main() {
> accumulator_set<double, features<tag::mean, droppable<tag::sum>
> > > accum_set;
> accum_set(1.0);
> accum_set(2.0);
> accum_set.drop<tag::sum>();
> accum_set(3.0);
> std::cout << mean(accum_set) << std::endl; //prints 1
> }
> This behavior can easily cause surprises. It must
> be fixed to print 2. I'm not quite sure how to implement
> it correctly. I think that probably you need to store the
> result at the point that drop is called for external lookup
> and continue to collect data for internal usage. Of course,
> this only applies in the face of dependencies.

The code is doing something a little unusual, but the result looks
correct to me. What do you think the result should be, and why?

> The user guide needs to specify what header each component is in.


> There is no description of what this macro is for.


> accumulator_set.hpp line 149
> corresponds to -- What?

... a feature. Fixed.

> accumulators_fwd.hpp #define's
> This is a very bad idea as it might make
> make the value smaller than it would otherwise
> have been. Please just check FUSION_MAX_VECTOR_SIZE
> and issue a #error directive if it is too small.

I had it that way at first, but changed it due to user feedback. The
idea is to make it so that you can just #define
BOOST_ACCUMULATORS_MAX_FEATURES and not have to worry about configuring

> For simple cases at least the library seems
> to cause little to no overhead with msvc compared to
> hard coding everything.

Thanks for the report!

> The docs need to state clearly what happens when
> more than one accumulator maps to the same feature.

Right. The first accumulator with a particular feature is the one that
gets used. Any others with the same feature are ignored.

> Overall the users guide is quite good.
> I had no difficulty understanding it.
> The reference manual is pretty sketchy though.

Good docs are always the hardest part.

> For the most part the interface is clean and easy to use.
> I definitely think Accumulators should be included in boost.


Eric Niebler
Boost Consulting

Boost list run by bdawes at, gregod at, cpdaniel at, john at