dolfin team mailing list archive

["Garth N. Wells" <gnw20@xxxxxxxxx>]

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
> > > > > > --
> > > > > > 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

I just made the change, but please contribute a demo.

Garth

> /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