On Mon, Feb 16, 2009 at 09:49:49PM +0000, A Navaei wrote: > 2009/2/16 Anders Logg <[email protected]>: > > On Mon, Feb 16, 2009 at 09:37:38PM +0000, A Navaei wrote: > >> 2009/2/16 Anders Logg <[email protected]>: > >> > On Mon, Feb 16, 2009 at 09:15:21PM +0000, A Navaei wrote: > >> >> 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? > >> > > >> > The Function class has been designed for extension in one particular > >> > way, namely by overloading the eval() function. However, I'm starting > >> > to believe it might make sense to make the two member variables in > >> > Function protected. Would that be enough? I still don't think it's a > >> > good idea to do the same for FunctionSpace as there are quite a few > >> > more member variables. > >> > >> If it doesn't mess up other dependencies, that would be indeed helpful. > > > > ok. I'll wait for Garth to comment before I make the changes. > > > >> Also, I'm curious to know why some variables are returned by value and > >> some by reference, eg, function_space() and function_space_ptr(), and > >> why the same thing happens for some ctors, eg, Function(const > >> FunctionSpace V&) and Function(boost::shared_ptr< FunctionSpace > V). > > > > Which are returned by value? Everything that is not a built-in type > > (int, float etc) is returned by reference to avoid copying. > > I mean this: > > /// Return the function space > const FunctionSpace& function_space() const; > > /// Return the function space > boost::shared_ptr<const FunctionSpace> function_space_ptr() const; > > Is this for compatibility with codes written prior to the appearance > of shared pointers?
It's to provide a choice between the two. References are supported natively by C++ and allows cleaner, nicer, simpler syntax than shared pointers so this is the default. But when shared pointers are needed one can choose to use those. -- Anders
signature.asc
Description: Digital signature
_______________________________________________ DOLFIN-dev mailing list [email protected] http://www.fenics.org/mailman/listinfo/dolfin-dev
