From: Lewis Hyatt (lhyatt_at_[hidden])
Date: 2007-08-07 16:46:55
Hi, here are my quick thoughts on this library. I don't have time for
much more than this... sorry it isn't more thorough.
* What is your evaluation of the design?
I guess my first impression was "way too many concepts", but after a
while it all starts to make sense. It seems pretty flexible what you can
do with it. It looks quite daunting at first to write new series
classes, but that probably isn't going to be that common a task. Writing
new algorithms seems pretty straightforward and I wouldn't mind trying
to add some new ones.
* What is your evaluation of the potential usefulness of the library?
I don't have a current use for it, but I can see how a lot of people would!
* Did you try to use the library? With what compiler? Did you
have any problems?
cygwin only has boost 1.33 still, and I didn't have time to get 1.34.1,
so I was missing too much stuff to compile the library.
* How much effort did you put into your evaluation? A glance? A
quick reading? In-depth study?
About an hour reading the docs and looking at the examples.
* Are you knowledgeable about the problem domain?
More or less. I do a lot of data analyis with std::vector and legacy C
libraries like clapack.
Here are some random comments:
-I don't like the names "daily", "weekly", etc, because they give the
impression that the library is only meant for daily data. I think it
would be better just to define a template stride<> inheriting from
mpl::int_<>, so you could write instead something like
dense_series<double, stride<7> >
and the meaning of 7 would depend on the context of the program.
-I am confused by the statement in the docs that "A time series is a
series of data points, sampled at regular intervals". If that's true,
then what does it mean to use a floating point offset? Doesn't that mean
I can use irregular intervals too? I was thinking it would be natural to
have an FFT algorithm for a dense time series, but that would only make
sense if it was sampled at regular intervals. I guess a dense series
always has an integral offset type, so it would make sense there.
-I worry a little about the dual interfaces for ordered_inserter, maybe
it should be split into two classes? Especially for the output iterator
usage, I think people would frequently try to use it with std::copy and
then forget to call commit(), causing silent errors. Maybe that's just
something the user will have to get used to. Another option could be to
use a new class called ordered_insert_iterator with a shared_ptr pImpl
setup, so that the last temporary object would call commit() from its
destructor after the algorithm completed. In that case, problems would
be caused if people created non-temporary objects, but I think in
practice, in the majority of cases, the output iterator would be used by
creating a temporary object and passing it to std::copy or whatever.
-I think invert_elements() should have a different name. For one thing,
there is confusion with invert_heaviside(), which is completely
unrelated, and for another, this name could also work fine to describe a
function which multiplied every element by -1. I can't think of a decent
* Do you think the library should be accepted as a Boost library?
Given that I didn't have time to look into this much more, I'm not sure
how seriously my opinion can be taken, but I do vote yes.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk