Thread Previous • Date Previous • Date Next • Thread Next |
Anders Logg wrote:
On Mon, Jun 23, 2008 at 08:03:35PM +0100, Garth N. Wells wrote:Anders Logg wrote: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?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. /DagTimeDependent 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! /DagI 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. GarthI can make the (trivial) change and add a demo under function/TimeDependent/cppI just made the change, but please contribute a demo. GarthI'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 '*'.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
Thread Previous • Date Previous • Date Next • Thread Next |