dolfin team mailing list archive

[Johan Hake <hake.dev@xxxxxxxxx>]

On 02/25/2013 09:56 AM, Garth N. Wells wrote:
> On 25 February 2013 08:45, Martin Sandve Alnæs <martinal@xxxxxxxxx> wrote:
>> Sounds confusing to me.
> 
> I'm confused. Is it a hack to get around a design flaw further up the chain?

Well, yes and no. It is for the SWIG interface.

When we return a cpp.Type from the dolfin cpp interface and we have
specialized its interface in the Python layer we need to be able to
return such a specialized type. This is most crucial for the whole adapt
chain.

For now we have solved it by (more or less like below)

%rename (_child) dolfin::Function::child

%extend dolfin::Function
{
def child(self):
    from dolfin.function import Function
    func = self._adapt()
    return Function(func.function_space(), func.vector())
}

Now Function.child return a python specialized Function.

> Does it have to be a constructor? What's wrong with
>> a shallow_copy() function?

With some __new__ magic we might get around with a member function. But
the __init__ method is already quite complex as it is. Not sure it will
be possible to combine that logic with a __new__ method...

Also if we want to correctly initiate the Function in its __init__
function, we, AFAIK, have to use the parent constructor for it to be
correctly initiated.

> No need for that either because one can already access shared pointers
> to the function space and vector.

This is what we do today. It works fine, but all hierarchical
information is lost. To retain that we need the actual Function too.

> I like to avoid shallow copies. It gets confusing as objects have more
> data and increases the likelihood that a code change will have
> knock-on effects.

I see this and my suggestion using shared_ptr is extremely error prone
for a C++ user using shared_ptr to store all objects.

What with introducing an explicit constructor like:

  Function(const Function&, bool shallow)

without default argument, we could most probably get rid of the
FunctionSpace and GenericVector constructor.

Johan

> 
> Garth
> 
>> Martin
>>
>>
>> On 22 February 2013 14:25, Johan Hake <hake.dev@xxxxxxxxx> wrote:
>>>
>>> On 02/22/2013 02:18 PM, Garth N. Wells wrote:
>>>> On 22 February 2013 13:13, Johan Hake <hake.dev@xxxxxxxxx> wrote:
>>>>> Any comment on this?
>>>>>
>>>>> Basically we need another constructor which generates a shallow copy of
>>>>> a Function, which also has access to the original Function.
>>>>>
>>>>
>>>> Would this be a non-issue if using a shared_ptr? What's the difference
>>>> between a shallow copy of a Function and a pointer to a Function?
>>>
>>> The shared_ptr is just a way to differ a deep copy from a shallow copy.
>>> Using a shared_ptr is really just an abuse of syntax. The shared_ptr
>>> will not be passed to the new Function anyhow. There might be other ways
>>> to differentiate these two constructors from each others. The obvious
>>> being:
>>>
>>>   Function(const Function& other, bool deepcopy)
>>>
>>> but not sure that is any better...
>>>
>>> The reason we need this in the first place is to be able to generate a
>>> Python wrapped version of a Function when such is returned from the C++
>>> interface. With a shallow copy constructor we minimize the amount of
>>> copying.
>>>
>>> Johan
>>>
>>>> Garth
>>>>
>>>>> Johan
>>>>>
>>>>> On 02/21/2013 07:08 PM, Johan Hake wrote:
>>>>>> Hello!
>>>>>>
>>>>>> To fix:
>>>>>>
>>>>>> [child().child() fails for Function and FunctionSpaces in Python]
>>>>>> https://bugs.launchpad.net/bugs/1130354
>>>>>>
>>>>>> we need to add a way of shallow copy a Function. For now we have used:
>>>>>>
>>>>>> Function(boost::shared_ptr<const FunctionSpace> V,
>>>>>>          boost::shared_ptr<GenericVector> x);
>>>>>>
>>>>>> Which create a new Function with a shared FunctionSpace and
>>>>>> GenericVector. The problem is that there is no way of bringing
>>>>>> Hierarchical data to the new Function as it is not passed through the
>>>>>> constructor.
>>>>>>
>>>>>> We would need something like:
>>>>>>
>>>>>> Function(boost::shared_ptr<const Function> u);
>>>>>>
>>>>>> which is interpreted as a shallow copy.
>>>>>>
>>>>>> We could then map this to:
>>>>>>
>>>>>>   Function._shallow_copy()
>>>>>>
>>>>>> and
>>>>>>
>>>>>>   Function(const Function& u);
>>>>>>
>>>>>> to
>>>>>>
>>>>>>   Function._deep_copy()
>>>>>>
>>>>>> in the Python interface. Then we add:
>>>>>>
>>>>>>   Function.
>>>>>>
>>>>>>   def copy(self, deepcopy=True):
>>>>>>      if deepcopy:
>>>>>>         return self._deep_copy()
>>>>>>      else:
>>>>>>         return self._shallow_copy()
>>>>>>
>>>>>> Any objections to adding:
>>>>>>
>>>>>>   Function(boost::shared_ptr<const Function> u);
>>>>>>
>>>>>> as a shallow copy constructor?
>>>>>>
>>>>>> If so we could maybe remove:
>>>>>>
>>>>>>    Function(boost::shared_ptr<const FunctionSpace> V,
>>>>>>             boost::shared_ptr<GenericVector> x);
>>>>>>
>>>>>> as I think we added it so we could create a shallow copy in the Python
>>>>>> interface.
>>>>>>
>>>>>> Johan
>>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Mailing list: https://launchpad.net/~dolfin
>>>>> Post to     : dolfin@xxxxxxxxxxxxxxxxxxx
>>>>> Unsubscribe : https://launchpad.net/~dolfin
>>>>> More help   : https://help.launchpad.net/ListHelp
>>>
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~dolfin
>>> Post to     : dolfin@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~dolfin
>>> More help   : https://help.launchpad.net/ListHelp
>>
>>