Re: [ublas-dev] Re: [boost] Re: Boost Mathematicians

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)
_______________________________________________