Boost logo

Ublas :

Subject: Re: [ublas] banded_matrix and bindings
From: Thomas Klimpel (Thomas.Klimpel_at_[hidden])
Date: 2008-12-01 18:08:36

Hi Vardan,

Vardan Akopian wrote:
> 1) As I pointed out here
> BOOST_UBLAS_OWN_BANDED must not be defined for this binding to work.
> So perhaps this should be checked in the binding and #error out.

Good idea.

> 2) Related to the same flag, it seams to me that banded_matrix has
> some inconsistent code. In the functions that need to access elements
> of the matrix the code is branched based on this flag. However the
> constructor for banded_matrix is not - it always allocates
> std::max(size1, size2)*(lower + 1 + upper). This strikes me as
> inconsistent, wasting memory in some cases (when the flag is not
> defined and the number of columns is larger than the number of rows)
> and potentially confusing for the binding (see 3 below).

I will note to have a look at it later, and open a track ticket in case I come to the conclusion that it is indeed inconsistent.

> 3) The documentation
> ( ) requires
> that AB's dimension be (LDAB,N), where N is the number of columns in A
> (M,N). In case if M > N, banded_matrix will actually allocate LDAB*M
> memory (instead of LDAB*N). Fortunately, because LDAB is the leading
> dimension, this is ok (it only wastes memory).

I guess allocating too much memory is always erring on the safe side, but wasting memory.

> 4) The current signature of the gbtrf() function does not allow to
> check if the proper banded matrix was allocated. For example if a
> matrix with 2 lower and 3 upper allocated diagonals is passed to
> gbtrf(), then it assumes that the actual matrix it's dealing with, has
> 2 lower and 1 upper diagonal. This does not provide any help with user
> errors. In principle we could modify the signature of the function,
> and require the actual number of upper diagonals (ku) to also be
> passed in. Then we could do all the necessary checks to ensure that a
> proper (larger) matrix is allocated.

I'm not convinced.

> Finally, I'm wandering what's the reason for the
> BOOST_UBLAS_OWN_BANDED implementation? Is there any 3rd party library
> that takes advantage of this. Or is there another reason for which
> this approach is better? I'm tempted to propose the remove it.

Good question. The name "BOOST_UBLAS_OWN_BANDED" indicates to me that there are no 3rd_party libraries taking advantage of this. Again, opening a track ticket might be an option.