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 > > [email protected] > > http://www.fenics.org/mailman/listinfo/dolfin-dev > > > _______________________________________________ > DOLFIN-dev mailing list > [email protected] > http://www.fenics.org/mailman/listinfo/dolfin-dev
signature.asc
Description: Digital signature
_______________________________________________ DOLFIN-dev mailing list [email protected] http://www.fenics.org/mailman/listinfo/dolfin-dev
