2009/2/16 Anders Logg <[email protected]>: > On Mon, Feb 16, 2009 at 09:59:01PM +0100, Martin Sandve Alnæs wrote: >> On Mon, Feb 16, 2009 at 9:39 PM, Anders Logg <[email protected]> wrote: >> > On Mon, Feb 16, 2009 at 06:11:41PM +0000, A Navaei wrote: >> >> 2009/2/16 A Navaei <[email protected]>: >> >> > 2009/2/16 Anders Logg <[email protected]>: >> >> >> On Mon, Feb 16, 2009 at 05:36:48PM +0000, Garth N. Wells wrote: >> >> >>> >> >> >>> >> >> >>> A Navaei wrote: >> >> >>> > 2009/2/16 Anders Logg <[email protected]>: >> >> >>> >> On Mon, Feb 16, 2009 at 04:36:21PM +0000, A Navaei wrote: >> >> >>> >>> 2009/2/15 Anders Logg <[email protected]>: >> >> >>> >>>> On Sat, Feb 14, 2009 at 06:44:05PM +0000, Garth N. Wells wrote: >> >> >>> >>>>> >> >> >>> >>>>> A Navaei wrote: >> >> >>> >>>>>> [snip] >> >> >>> >>>>>>>>> Function should not change the FunctionSpace (that's why >> >> >>> >>>>>>>>> FunctionSpace is >> >> >>> >>>>>>>>> const). FunctionSpace shouldn't depend on the data and its >> >> >>> >>>>>>>>> size should be >> >> >>> >>>>>>>>> defined when creating the FunctionSpace. >> >> >>> >>>>>>>> The same applies for FunctionSpace as its member variables >> >> >>> >>>>>>>> are private >> >> >>> >>>>>>>> and the public accessors are read-only which. Consider a >> >> >>> >>>>>>>> sub-class >> >> >>> >>>>>>>> ImageFunctionSpace:FunctionSpace, with a constructor like: >> >> >>> >>>>>>>> >> >> >>> >>>>>>>> ImageFunctionSpace(ImageType *imagePtr) >> >> >>> >>>>>>>> >> >> >>> >>>>>>>> where imagePtr is supposed to initialise >> >> >>> >>>>>>>> FunctionSpace::_mesh using >> >> >>> >>>>>>>> the image size, and then _dofmaps itself is initialised >> >> >>> >>>>>>>> using _mesh. >> >> >>> >>>>>>>> How would you do that considering the restrictions? >> >> >>> >>>>>>>> >> >> >>> >>>>>>> The FunctionSpace has pointers to the mesh, etc. You just >> >> >>> >>>>>>> need to create >> >> >>> >>>>>>> your mesh and pass it to the FunctionSpace constructor. What >> >> >>> >>>>>>> else you then >> >> >>> >>>>>>> do with the mesh, etc is up to you. >> >> >>> >>>>>> That can be done _outside_ of a sub-class. A sub-class of >> >> >>> >>>>>> FunctionSpace doesn't have a control over _mesh of its own >> >> >>> >>>>>> parent >> >> >>> >>>>>> FunctionSpace. The following example may make this more clear: >> >> >>> >>>>>> >> >> >>> >>>>>> template <typename TImage> >> >> >>> >>>>>> class DolfinImageFunctionSpace : public dolfin::FunctionSpace >> >> >>> >>>>>> { >> >> >>> >>>>>> public: >> >> >>> >>>>>> // just some itk typedefs -- ignore >> >> >>> >>>>>> typedef TImage ImageType; >> >> >>> >>>>>> typedef typename ImageType::PixelType PixelType; >> >> >>> >>>>>> typedef typename ImageType::SizeType SizeType; >> >> >>> >>>>>> >> >> >>> >>>>>> // .. and some dolfin typedefs -- ignore >> >> >>> >>>>>> typedef typename std::tr1::shared_ptr<const dolfin::Mesh> >> >> >>> >>>>>> MeshConstPointerType; >> >> >>> >>>>>> typedef typename std::tr1::shared_ptr<const >> >> >>> >>>>>> dolfin::FiniteElement> >> >> >>> >>>>>> ElementConstPointerType; >> >> >>> >>>>>> typedef typename std::tr1::shared_ptr<const dolfin::DofMap> >> >> >>> >>>>>> DofMapConstPointerType; >> >> >>> >>>>>> >> >> >>> >>>>>> // the ctor >> >> >>> >>>>>> DolfinImageFunctionSpace(ImageType* imageData, >> >> >>> >>>>>> >> >> >>> >>>>>> MeshConstPointerType mesh, >> >> >>> >>>>>> >> >> >>> >>>>>> ElementConstPointerType element, >> >> >>> >>>>>> >> >> >>> >>>>>> DofMapConstPointerType dofmap) : >> >> >>> >>>>>> dolfin::FunctionSpace(mesh, element, dofmap) >> >> >>> >>>>>> { >> >> >>> >>>>>> SizeType imageSize = >> >> >>> >>>>>> imageData->GetBufferedRegion().GetSize(); >> >> >>> >>>>>> >> >> >>> >>>>>> // here, we whish to call some thing like: >> >> >>> >>>>>> // _mesh = UnitSquare(imageSize[0], >> >> >>> >>>>>> imageSize[1]); >> >> >>> >>>>>> // but it's private and the accessor is >> >> >>> >>>>>> read-only. >> >> >>> >>>>>> }; >> >> >>> >>>>>> } >> >> >>> >>>>>> >> >> >>> >>>>> This breaks the concept of a function space. A function space >> >> >>> >>>>> is defined >> >> >>> >>>>> in terms of a mesh (and other things). A function space does >> >> >>> >>>>> not define >> >> >>> >>>>> its mesh. >> >> >>> >>>>> >> >> >>> >>>>> It looks to me like the class that you're creating should be >> >> >>> >>>>> called >> >> >>> >>>>> something like >> >> >>> >>>>> >> >> >>> >>>>> DolfinImageProblem, >> >> >>> >>>>> >> >> >>> >>>>> which can create its own Mesh, FunctionSpace and other objects. >> >> >>> >>>>> >> >> >>> >>>>> Garth >> >> >>> >>>>> >> >> >>> >>>>>> -Ali >> >> >>> >>>> I think what you need to do is something like this: >> >> >>> >>>> >> >> >>> >>>> 1. Create a subclass of Function >> >> >>> >>>> >> >> >>> >>>> 2. In the constructor of your Function, call the empty Function() >> >> >>> >>>> constructor >> >> >>> >>>> >> >> >>> >>>> 3. Then, still in the constructor of your Function (not >> >> >>> >>>> FunctionSpace), create everything necessary like figuring out the >> >> >>> >>>> mesh, dofmap etc and from that create a FunctionSpace >> >> >>> >>>> >> >> >>> >>>> 4. Then use that FunctionSpace (still in the constructor of your >> >> >>> >>>> Function subclass) to create a new Function v which uses your >> >> >>> >>>> special >> >> >>> >>>> FunctionSpace in its constructor >> >> >>> >>>> >> >> >>> >>>> 5. Finally, assign this function to the created Function: >> >> >>> >>>> >> >> >>> >>>> *this = v; >> >> >>> >>> error: no match for 'operator=' in >> >> >>> >>> '*(itk::DolfinImageFunction<itk::Image<double, 2u> >*)this = v' >> >> >>> >>> >> >> >>> >>> Assigning Function to ImageFunction is a trouble, see the full >> >> >>> >>> code here: >> >> >>> >>> >> >> >>> >>> http://code.google.com/p/wrapitk/source/browse/trunk/ExternalProjects/ItkDolfin/src/itkDolfinImageFunction.h#87 >> >> >>> >> Do you have the latest hg version? >> >> >>> >> >> >> >>> >> Function assignment should work (see recent discussion on the >> >> >>> >> mailing >> >> >>> >> list on copy constructors and assignment operators for Function). >> >> >>> >> >> >> >>> >>> I don't understand the philosophy behind this tight security: why >> >> >>> >>> no >> >> >>> >>> protected member variables? Why is everything either public or >> >> >>> >>> private? Needless to say, the protected members were designed to >> >> >>> >>> allow >> >> >>> >>> class extensions, by banning it, you're making sub classing a >> >> >>> >>> unnecessarily complicated task. The above problem has it's own >> >> >>> >>> work >> >> >>> >>> arounds, but I don't understand why the obvious way is blocked. >> >> >>> >> The reason is simply that the constructor arguments in Function and >> >> >>> >> FunctionSpace are const. This means no one is allowed to change the >> >> >>> >> variables, not even the Function class (or FunctionSpace class) >> >> >>> >> itself. For example, it's reasonable when you create a Function on >> >> >>> >> a >> >> >>> >> FunctionSpace that the Function will not change the FunctionSpace. >> >> >>> > >> >> >>> > Whether the member variables are read-only or not, and their >> >> >>> > visibility are two separate concepts - you can have any combination >> >> >>> > of >> >> >>> > these. Function and FunctionSpace do change their const variables >> >> >>> > anyway, eg, in operator= you have: >> >> >>> > >> >> >>> > // Assign vector >> >> >>> > init(); >> >> >>> > dolfin_assert(_vector); >> >> >>> > *_vector = *v._vector; >> >> >>> > >> >> >>> > Which is in contradiction with what you wrote about not even the >> >> >>> > class >> >> >>> > itself cannot change the variables. >> >> >>> > >> >> >>> >> >> >>> Anders was referring to FunctionSpace. Obviously, the vector >> >> >>> associated >> >> >>> with a discrete Function cannot be const (and it isn't). >> >> >>> >> >> >>> > It would be rare that all classes have to have read-only inputs, >> >> >>> > where >> >> >>> > the sub-classes are setences to inherit the same properties. Now, I >> >> >>> > simply cannot have the right operator= assinging Function to >> >> >>> > ImageFunction implemented, since I cannot assign anything to the >> >> >>> > private member variables. >> >> >>> > >> >> >>> > If the visibility of the members variables is not going to change to >> >> >>> > protected, or at least a protected read/write accessor is not >> >> >>> > provided, then there will be no option but implementing everything >> >> >>> > in >> >> >>> > a third-party class faking the current ImageFunction, as it's not >> >> >>> > going to be derived from Function. >> >> >>> > >> >> >>> >> >> >>> All you need to do is create a suitable FunctionSpace before creating >> >> >>> your Function. >> >> >>> >> >> >>> Garth >> >> >> >> >> >> Yes, and you should be able to create that FunctionSpace inside the >> >> >> constructor for your Function subclass. >> >> > >> >> > Yes, I can _create_ it, but I cannot _assign_ it :) >> >> > >> >> > Now I'm trying this work around: >> >> > >> >> > DolfinImageFunction(ImageType* imageData) : >> >> > dolfin::Function() >> >> > { >> >> > ... >> >> > // Create a Function instance >> >> > FSConstPointerType V = CreateFunctionSpace(); >> >> > DolfinImageFunction v(V); >> >> > *this = v; >> >> > }; >> >> > >> >> > which requires the repsence of this ctor: >> >> > >> >> > DolfinImageFunction(boost::shared_ptr<const dolfin::FunctionSpace> V): >> >> > dolfin::Function(V) >> >> > { >> >> > }; >> >> > >> >> > This might work. >> > >> > Yes, that's what I suggested (but maybe I didn't explain it too well). >> > >> >> The above builds, but then I get thos runtime error: >> >> >> >> RuntimeError: *** Error: Cannot copy Functions which do not have a >> >> Vector (user-defined Functions). >> >> >> >> The error originates from 'this' instance and not from v. >> >> >> >> Just in case, here is the full code: >> >> >> >> http://code.google.com/p/wrapitk/source/browse/trunk/ExternalProjects/ItkDolfin/src/itkDolfinImageFunction.h#47 >> > >> > Yes, this is expected and in accordance with another thread today on >> > assignment of Functions. It is not possible to assign Functions which >> > are only defined by an eval() operator (like in your case). >> > >> > The real problem here is that C++ does not allow anything to happen >> > between the call to the contstructor and the initialization of the >> > base class. What you really want to do is this: >> > >> > DolfinImageFunction(ImageType* imageData) >> > { >> > // Create function space >> > FSConstPointerType V = CreateFunctionSpace(imageData); >> > >> > // Initialize base class >> > Function(V); >> > } >> > >> > This is possible in Python but not C++ as the call to the base class >> > constructor must happen before the constructor body. >> > >> > The only solution I see is to add an init() function to Function that >> > allows a user to set the FunctionSpace. Either we name it init() as >> > we've done in many other DOLFIN classes, or we add a function named >> > set_function_space() which would match has_function_space(). >> >> You still haven't explained why making member variables protected >> instead of private is not an option, as was asked for? >> Might be a good reason, but I'm curious too what that reason is. > > I'd like all member variables to be private unless there's a very good > reason not to make them private.
Here is one good reason: to allow others to extend the class :) > The Function/FunctionSpace classes > are particularly intricate with a _vector that may or may not be null > depending on the current state of the Function, so by allowing access > only through public functions we can perform checks (which we could do > better) and some magic. For example, calling vector() does not only > return the vector instance, it also creates the vector and initializes > it with the correct size if necessary. Why not following the standard design pattern of accessors used by many other people? -Ali > > -- > Anders > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.9 (GNU/Linux) > > iEYEARECAAYFAkmZ1hUACgkQTuwUCDsYZdGcBgCdH2yypFBvkAv/IGpu71Ci7OLR > Xh8An3vazHPPvMg3PYYD6qjrqmr5lFxA > =PQQ/ > -----END PGP SIGNATURE----- > > _______________________________________________ > DOLFIN-dev mailing list > [email protected] > http://www.fenics.org/mailman/listinfo/dolfin-dev > > _______________________________________________ DOLFIN-dev mailing list [email protected] http://www.fenics.org/mailman/listinfo/dolfin-dev
