← Back to team overview

dolfin team mailing list archive

Re: [HG DOLFIN] Remove functions copy() and create() (use factory instead)

 

> 2008/4/3, kent-and@xxxxxxxxx <kent-and@xxxxxxxxx>:
>> > 2008/4/3, Anders Logg <logg@xxxxxxxxx>:
>>  >> On Thu, Apr 03, 2008 at 07:47:13PM +0200, Martin Sandve Alnæs wrote:
>>  >>  > We only talked about create() the other day, copy() can still be
>>  >>  > useful! Or does the factory duplicate this functionality?
>>  >>
>>  >>
>>  >> No, but Kent has added an assignment operator:
>>  >>
>>  >>   GenericVector* y = x.factory().createVector();
>>  >>   *y = x;
>>  >
>>  > Ok, but the assignment operator doesn't exist in Python, so some
>>  > variant of the copy function may still be needed. Could be just a
>>  > wrapper for the two lines above.
>>
>>
>>
>> PyDolfin has assign:
>>  dolfin_function_pre.i:%rename(assign) dolfin::Function::operator=;
>>
>>
>>
>>
>>  >
>>  > But the current default implementation of operator= (just return
>>  > *this;) isn't good, it should be abstract or at least raise an error
>>  > somehow.
>>  >
>>
>>
>> It did not compile when it was defined as an abstract function. But I
>>  agree completely. You probably have a fix, I didn't manage.
>
> I didn't know virtual operators were allowed at all in C++, but simply
> calling dolfin_error in there is one solution (I do this in Python
> some times to "simulate" pure virtual functions).
>

Then I get a warning:

cc1plus: warnings being treated as errors
./dolfin/la/GenericVector.h: In member function ‘virtual const
dolfin::GenericVector& dolfin::GenericVector::operator=(const
dolfin::GenericVector&)’:
./dolfin/la/GenericVector.h:111: warning: no return statement in function
returning non-void
scons: *** [dolfin/elements/ProjectionLibrary.os] Error 1
scons: building terminated because of errors.



>
>>  > And I the uBlas implementation seems potentially dangerous too,
>>  >
>>  > const uBlasVector& uBlasVector::operator= (const GenericVector& x_)
>>  > {
>>  >   const uBlasVector* x = dynamic_cast<const uBlasVector*>(&x_);
>>  >   if (!x) error("The vector should be of type PETScVector");
>>  >
>>  >   *this = (*x)*1.0;
>>  >   return *this;
>>  > }
>>  >
>>  > In particular, this line
>>  >   *this = (*x)*1.0;
>>  > depends on (*x)*1.0 being resolved by the ublas_vector subclass,
>>  > triggering operator= in ublas_vector. Implementing operator* in
>>  > GenericVector will break this and possibly make an infinite
>> recursion.
>>  > Unless I misunderstand something? (I don't know ublas).
>>  >
>>
>>
>> Please help here as well. The reason it looks like this is that I don't
>>  know ublas.
>>
>>  Kent
>
> Plus the use of multiple inheritance... Maybe uBlasVector should be
> changed to own an ublas_vector instead of being one? If operators are
> added to GenericVector, multiple inheritance will be messy.

But GenericVector is abstract so I guess the operators should basically
be wrappers on top the concrete vectors like the ublas vector.

However,  I agree that things are getting a bit messy.

Kent




Follow ups

References