← Back to team overview

dolfin team mailing list archive

Re: TimeDependent is too clever!

 



Anders Logg wrote:
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.


BTW, it's currently used by two demos.

Garth



------------------------------------------------------------------------

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


References