Boost logo

Boost :

From: Jeremy Graham Siek (jsiek_at_[hidden])
Date: 2004-07-05 15:43:59


Hi Toon,

On Jul 5, 2004, at 10:54 AM, Toon Knapen wrote:
> I think Dave's evaluation of uBlas should be closely looked at and we
> should address the issues mentioned. So I would like to comment so
> that
> we can further these isues and really tackle these problems.
>
>
> David Abrahams wrote:
>
> > Jeremy and I have just completed a re-evaluation of uBlas based on
> > what's in uBlas' own CVS repository, having not discovered that
> until
> > recently (you should have that info on uBlas' Boost page!)  We have
> > some major complaints with uBlas' design.  The list is long, and the
> > issues run deep enough that we don't believe that uBlas is a
> suitable
> > foundation for the work we want to do.
> I however regret that you never provided your feedback before.

I would have like to, but was swamped with other work when uBLAS went
through review (I was probably writing the BGL book).

> >
> > Here is a partial list of things we take issue with:
> >
> > Interface Design
> > ----------------
> >
> > * Not grounded in Generic Programming.  The concept taxonomies, to
> the
> >   extent they exist, are weak, and poorly/incorrectly documented.
> >   Aspects of the design that should be generic are not (e.g. only
> >   certain storage containers are supported, rather than supplying
> >   storage concept requirements).  No linear algebra concepts (vector
> >   space, field, etc.)  The library is so out-of-conformance with our
> >   expectations for Generic Programming that this one item by itself
> >   is probably enough to make it unsuitable for us.
> The problem regarding documentation regularly comes up on the ml. Some
> people started with an alternative documentation for uBlas. Maybe it's
> time to merge and rework all uBlas documentation. And indeed
> documentation is crucial for developing generic concepts.
>
> >
> > * Redundant specification of element type in matrix/vector storage.
> Could you elaborate ?
>
> >
> > * size1 and size2 should be named num_rows and num_columns or
> >   something memnonic
> This is just a matter of documentation. AFAIK there is no accepted
> concept that demands 'num_rows' and 'num_columns'. The '1' and '2'
> refer
> to the first and second index which is also a nice convention IMO.

No, there are uBLAS concepts that refer to size1 and size2. For example,
the Matrix Expression concept.

> >
> > * iterator1 and iterator2 should be named column_iterator and
> >   row_iterator or something memnonic.
> same as above.
>
> >
> > * prod should be named operator*; this is a linear algebra library
> >   after all.
> This also came up a few times and we never agreed how users could
> clearly distinguish the product and the element-product so there was
> decided to use more explicit function-names.

element-wise products do not play the same central role that products do
in linear algebra. Thus, operator* should be used for product, and some
other name, like element_prod for element-wise product.

> But the most important thing about operator-overloading is being able
> to
> use generic algorithms that exploit the given operator. Not having
> operator* AFAIK never prohibited the reuse of generic LA algorithms.
>
> >
> > * begin() and end() should never violate O(1) complexity
> expectations.
> That would indeed be ideal but why exactly do you insist on O(1)
> complexity.

Because the C++ Standard requirements for Container say constant time,
it's
what people expect, and because it follows the STL design style
(e.g., std::list has no operator[]).

> >
> > * insert(i,x) and erase(i) names used inconsistently with standard
> library.
> agreed. Again this could be solved by generating proper documentation
> that is also inline with the documentation of the standard library.

This is an interface change, so code would need to change too.

> >
> > * Matrix/Vector concept/class interfaces are way too "fat" and need
> to
> >   be minimized (e.g. rbegin/rend *member* functions should be
> >   eliminated).
> agreed.
>
> >
> > * The slice interface is wrong; stride should come last and be
> >   optional; 2nd argument should be end and not size; then a separate
> >   range interface could be eliminated.
> I'm not convinced slice and range could be united. A range is a
> special
> case that can more easily optimised compared to the slice
> implementation.
>
> >
> > * No support for unorderd sparse formats -- it can't be made to fit
> >   into the uBlas framework.
> I'm not qualified to comment here.
>
> >
> > Implementation
> > --------------
> >
> > * Expressions that require temporaries are not supported by uBLAS
> >   under release mode. They are supported under debug mode.  For
> >   example, the following program compiles under debug mode, but not
> >   under release mode.
> >
> >   #include <boost/numeric/ublas/matrix.hpp>
> >   #include <boost/numeric/ublas/io.hpp>
> >
> >   int main () {
> >       using namespace boost::numeric::ublas;
> >       matrix<double> m (3, 3);
> >       vector<double> v (3);
> >       for (unsigned i = 0; i < std::min (m.size1 (), v.size ()); ++
> i) {
> >         for (unsigned j = 0; j < m.size2 (); ++ j)
> >             m (i, j) = 3 * i + j;
> >         v (i) = i;
> >       }
> >       std::cout << prod (prod(v, m), m) << std::endl;
> >   }
> >
> >   The workaround to make it compile under release mode is to
> >   explicitly insert the creation of a temporary:
> >
> >       std::cout << prod (vector<double>(prod(v, m)), m) <<
> std::endl;
> >
> >   There should be no such surprises when moving from debug to
> >   release.  Debug mode should use expression templates, too, as the
> >   differences can cause other surprises.
> Agreed. The semantics in debug and release mode should be identical.
>
> >
> > * Should use iterator_adaptor. There is a ton of boilerplate
> iterator
> >   code in the uBLAS that needs to be deleted.
> uBlas originates from before the new iterator_adaptor lib. But indeed
> we
> might need to look at using the iterator_adaptor library.
>
> >
> > * Should use enable_if instead of CRTP to implement operators. uBLAS
> >   avoids the ambiguity problem by only using operator* for
> >   vector-scalar, matrix-scalar ops, but that's only a partial
> >   solution.  Its expressions can't interact with objects from other
> >   libraries (e.g. multi-array) because they require the intrusive
> CRTP
> >   base class.
> True but operator* between uBlas classes and multi_arrays are used
> very
> frequently.
>
> >
> > Testing
> > -------
> > * There should be a readme describing the organization of the tests.
> Indeed but this is a problem that can be solved (but must be dealed
> with)
> >
> > * Tests should not print stuff that must be manually inspected for
> >   correctness.
> Does it really bother you ?

Yes, very much. It should bother you too.

> >
> > * Test programs should instead either complete successfully
> >   (with exit code 0) or not (and print why it failed).
> agreed.
>
> >
> > Documentation
> > -------------
> >
> > * In really bad shape.  Redundant boilerplate tables make the head
> >   spin rather than providing useful information.
> >
> > * Needs to be user-centered, not implementation centered.
> >
> > * Need a good set of tutorials.
> >
> all agreed. Man, we really need to improve our documentation!
>
_______________________________________________
Jeremy Siek <jsiek_at_[hidden]>
http://www.osl.iu.edu/~jsiek
Ph.D. Candidate, Indiana University Bloomington
C++ Booster (http://www.boost.org)
_______________________________________________


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk