dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Mon, Jun 23, 2008 at 08:03:35PM +0100, Garth N. Wells wrote:
> 
> 
> Anders Logg wrote:
> > On Mon, Jun 23, 2008 at 07:31:09PM +0100, Garth N. Wells wrote:
> >>
> >> Dag Lindbo wrote:
> >>>> 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
> >>>>>>>>
> >>>>>>>> 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
> >>>
> >> I just made the change, but please contribute a demo.
> >>
> >> Garth
> > 
> > I'm not sure I like it but we can discuss it more later:
> > 
> > 1. I think it should be a reference. The '&' signifies that we
> > may access the address of the object just as much as '*'.
> >
> 
> I'm rather indifferent. I like the look of 'real& t' (which is why I 
> originally did it that way), but accept that we generally pass scalars 
> by value throughout the code which may cause some confusion. Do you 
> suggest that we leave it as it was?

No, I suggest we leave it like it is. I expect we will have some use
for TimeDependent in the future, possibly in connection with the new
TimeSeries class, and might want to make further modifications then.

-- 
Anders

> > 2. It's a bit strange to first send in an object to the constructor
> > and then call sync with a possibly new object. 
> 
> It's not intended to work this way. sync is for when the time has not 
> been declared when an object which depends on time is created.
> 
> Garth
> 
> To me, sync() should
> > take the time t by value (not reference).
> >
> 
> 
> 
> > Anyway. I'll make the new release as soon as the buildbots are happy
> > (currently building).
> > 
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > _______________________________________________
> > 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

Attachment: signature.asc
Description: Digital signature