Boost logo

Boost :

From: Eric Niebler (eric_at_[hidden])
Date: 2007-01-31 13:27:05

Steven Watanabe wrote:
> Eric Niebler wrote:
>> Hi, Steven,
>> Steven Watanabe wrote:
>>> 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. ;-)
> unless I missed something
> libs/accumulators/doc/../../libs/ == libs/libs/
> Which I think is not what you intend. Somehow in the user_s_guide.html this
> becomes libs/accumulators/libs/ I'm not quite sure whether
> this is a quickbook bug or the file was built as though it were in
> libs/accumulators/doc/html/

I think it's correct if the docs are built from the top-level
$BOOST_ROOT/doc directory.

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

>> 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.
> I'm not suggesting that you reinvent the
> fusion algorithms. I just think that fusion::vector
> is not necessarily a good sequence to use
> when the constructor parameters are all identical.
> Also if you write the sequence then you can
> guarantee the order of initialization.
>> 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.
> But EBCO can't help you anyway. Two distinct sub objects
> of the same type (accumulator_base) cannot share the same address.
> I assumed that you didn't want to rely on fusion initializing the elements
> in order, that was part of the reason I suggested rolling your own fusion
> sequence.

Hmm, you're right. post_construct should go away.

>>> 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 result should be 2 because 1.0 + 2.0 + 3.0 / 3 = 2.0
> Users of particular accumulators should not have to care about the
> internal dependencies. The mean should return the mean of all the
> values that have been added regardless of whether count/sum have
> been dropped.


>>> 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
>> Fusion.
> Consider.
> file a.cpp
> #include <boost/accumulators/accumulators.hpp>
> #include <boost/fusion/sequence/container/vector.hpp>
> file b.cpp
> #include <boost/fusion/sequence/container/vector.hpp>
> #include <boost/accumulators/accumulators.hpp>
> I'm not absolutely certain that this actually causes a problem
> with ODR, but even if it doesn't now it is very fragile.

Yikes. OK, this problem goes away if I abandon fusion::vector in favor
of an open-ended Fusion sequence that guarantees construction order. I
see Fusion cons<> is such a sequence. I might just switch to that and
kill 2 birds with one stone.

>>> 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.
> And the ones that are specified in the sequence are processed before those
> found indirectly.

Exactly. Thanks for your valuable feedback.

Eric Niebler
Boost Consulting

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