← Back to team overview

dolfin team mailing list archive

Re: TimeDependent is too clever!

 

>
>
> Anders Logg wrote:
>> On Mon, Jun 23, 2008 at 02:51:05PM +0200, Dag Lindbo wrote:
>>> Anders Logg wrote:
>>>> On Thu, Jun 19, 2008 at 11:41:52PM +0200, Dag Lindbo wrote:
>>>>> Hello again!
>>>>>
>>>>> Here comes another suggestion about the DOLFIN public interface:
>>>>>
>>>>> Constructing time dependent functions by subclassing from Function
>>>>> and
>>>>> TimeDependent is very very nice. The way u.sync(t) works by storing a
>>>>> pointer to t is also very clever. However, I think it is too clever
>>>>> --
>>>>> most would expect pass-by-value semantics when passing a scalar to a
>>>>> function. A very simple change could make it a lot clearer what's
>>>>> going
>>>>> on: Pass a pointer to t instead of a constant reference. This forces
>>>>> the
>>>>> user to take the address of the scalar, which really shows what's
>>>>> going
>>>>> on. E.g.
>>>>>
>>>>> u.sync(&t)
>>>>>
>>>>> Today I misunderstood the reference nature of sync(t), and wrote code
>>>>> like
>>>>> this:
>>>>>
>>>>> while(t < T){
>>>>>   "compute timestep"
>>>>>   t += dt
>>>>>   sync_bc(t)
>>>>> }
>>>>>
>>>>> void sync_bc(real t){
>>>>>   for "all Dirichlet BCs"
>>>>>      bc.sync(t)  // Pointer to stack memory gets assigned!!!!
>>>>> }
>>>>>
>>>>> :-( Of course, no one would be foolish enough to pass a pointer to
>>>>> stack
>>>>> memory knowingly in such a short lived context.
>>>>>
>>>>> /Dag
>>>> TimeDependent does not seem to be used anywhere in the code so now
>>>> it's just a fancy way of storing the current time. Objects that want
>>>> to store the current time could equally well just add a member
>>>>
>>>>   real t;
>>>>
>>>> I suggest we remove TimeDependent for now and add it back later when
>>>> we find any use for it.
>>>>
>>>> ok?
>>> No! It is really good feature for non-kernel applications.
>>> I think there is a clear benefit for users to have this base class to
>>> standardize things. It is one of only a handful situations where I
>>> really like multiple inheritance!
>>>
>>> /Dag
>>
>> I understand, but since the interface is not used in DOLFIN, it's
>> difficult to make any sense of how the class should behave.
>>
>> The current implementation is bound to cause trouble (as you have
>> noticed) and I don't know how to fix it since I don't know how the
>> class is used.
>>
>
> I think that the class is worth keeping and the simple change suggested
> by Dag should remove the ambiguity.
>
> Garth

I can make the (trivial) change and add a demo under
function/TimeDependent/cpp

/Dag

>
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> DOLFIN-dev mailing list
>> DOLFIN-dev@xxxxxxxxxx
>> http://www.fenics.org/mailman/listinfo/dolfin-dev
>
> _______________________________________________
> DOLFIN-dev mailing list
> DOLFIN-dev@xxxxxxxxxx
> http://www.fenics.org/mailman/listinfo/dolfin-dev
>




Follow ups

References