Re: [boost] Re: Boost Mathematicians

From: Toon Knapen (toon.knapen_at_[hidden])
Date: 2004-07-05 10:54:01

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.

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

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

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.

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

> 
> * 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!