Re: [Lapackpp-devel] bugs in operator= for LaIndex/LaGenMatComplex/LaGenMatDouble?

[Christian S. <sti...@tu...>]

Dear Brian,

Brian White schrieb:
> In LaIndex::oeprator=(const LaIndex &i), the following definition
> ignores the increment of the LaIndex it is copying:
> 
>         inline LaIndex& operator=(const LaIndex& i){
>             start_=i.start_; inc_=1; end_=i.end_;
>             return *this;}
> 
> I think inc_=1 should be inc_=i.inc_:

Yes, that's a bug. Thanks for reporting this. It has been fixed in CVS now.

> Also, LaGenMatComplex::operator=(COMPLEX s) and
> LaGenMatDouble::operator=(double s) seem to be problematic since both
> assign to the vectorDouble v without accounting for the active view
> of the LaGenMatDouble object.  I believe this means that if we assign
> (via operator=) through a submatrix view, the entire matrix will be
> effected.

Yes, that's a bug, too. I've fixed this by first checking for unit 
stride and then either assigning to the vectorDouble object (*much* 
faster!) or iterating over all elements.

> Below is some example code (taken essentially from the Lapack++ 1.1
> manual when it was still being developed by NIST/Tennessee/Oak Ridge).
> I've included what I believe to be the correct output and also
> the output I get before making the above changes.

Thanks for adding this. It helped a lot to verify the correct fix.

> By the way, this differs from the example in the manual only
> in so far as assignments are replaced by references.
> i.e., B = A(II,II) is replaced below by B.ref(A(II,II)).
> If I'm reading that manual correctly, B = A(II,II) should
> establish B as a reference to elements in A.  Instead it
> appears to perform a (deep) copy.

Yes, that's true and it's a feature :-) . Seriously, I've decided 
against that original lapackpp-1.1 approach (now also more clearly 
documented in the header) because of two reasons: 
LaGenMatDouble::LaGenMatDouble(const LaGenMatDouble &) [i.e. the copy 
constructor] was working as a by-value deep copy anyway. So having 
operator= [i.e. the copy assignment] work as a by-reference copy means 
that the copy constructor and the copy assignment were doing two 
different things, which is a bad thing. Secondly, many users of lapackpp 
got quite confused when they constructed a new LaGenMatDouble, assigned 
an initial value by the copy assignment (i.e. LaGenMatDouble B = A;) and 
to their surprise discovered that the newly created matrix won't behave 
as a standalone object but rather as a reference to some other object. 
If you want, you can say that the by-reference semantics were too 
difficult to grok for the average programmer. Therefore I changed the 
copy assignment to be the same as the copy constructor: a by-value deep 
copy. If you want a by-reference copy, use ref(). Those who use it are 
probably quite sure they know what they do.

Regards,

Christian