[kepler-dev] IOPortEvent bug
Christopher Brooks
cxh at eecs.berkeley.edu
Mon May 24 16:18:14 PDT 2010
Hi Colin,
A good test case would be one that would run in either Kepler or
Ptolemy and either in a release or from the devel trunk.
I realize that generating a test case is a bit of a chore. With
this error, I'm not sure if I could generate a test failure from
what I have from you.
I'm starting to think that "Cloning is considered harmful".
Edward and I continue to find cloning bugs. Our testing harness
identified a number of them, but I think we need more rigor somewhere.
For those just tuning in, if you have an actor that is either
missing a clone(Workspace) or is incorrectly handling fields or
types in the clone(Workspace) method, then you will see unusual
problems with Actor Oriented Classes.
Section 5.2.5 of the Volume 2 of the Ptolemy II design doc says:
--start--
All actors are cloneable. A clone of an actor needs to be a new instance of the same class, with the
same parameter values, but without any connections to other actors.
Consider the clone() method in figure 5.5, taken from the SimplerScale actor. This method begins
with:
SimplerScale newObject = (SimplerScale)super.clone(workspace);
The convention in Ptolemy II is that each clone method begins the same way, so that cloning works its
way up the inheritance tree until it ultimately uses the clone() method of the Java Object class. That
method performs what is called a “shallow copy,” which is not sufficient for our purposes. In particular,
members of the class that are references to other objects, including public members such as ports
public class SimplerScale extends Transformer {
...
public SimplerScale(CompositeEntity container, String name)
throws NameDuplicationException, IllegalActionException {
super(container, name);
output.setTypeAtLeast(input);
output.setTypeAtLeast(factor);
}
///////////////////////////////////////////////////////////////////
//// ports and parameters ////
/** The factor. The default value of this parameter is the integer 1. */
public Parameter factor;
///////////////////////////////////////////////////////////////////
//// public methods ////
/** Clone the actor into the specified workspace. This calls the
* base class and then sets the type constraints.
* @param workspace The workspace for the new object.
* @return A new actor.
* @exception CloneNotSupportedException If a derived class has
* has an attribute that cannot be cloned.
*/
public Object clone(Workspace workspace) throws CloneNotSupportedException {
SimplerScale newObject = (SimplerScale)super.clone(workspace);
newObject.output.setTypeAtLeast(newObject.input);
newObject.output.setTypeAtLeast(newObject.factor);
return newObject;
}
...
}
FIGURE 5.5. Code segment from the SimplerScale actor, showing the clone() method.
and parameters, are copied by copying the references. The NamedObj and TypedAtomicActor base
classes implement a “deep copy” so that all the contained objects are cloned, and public members reference
the proper cloned objects2.
Although the base classes neatly handle most aspects of the clone operation, there are subtleties
involved with cloning type constraints. Absolute type constraints on ports and parameters are carried
automatically into the clone, so clone() methods should never call setTypeEquals(). However, relative
type constraints are not cloned automatically because of the difficulty of ensuring that the other object
being referred to in a relative constraint is the intended one. Thus, in figure 5.5, the clone() method
repeats the relative type constraints that were specified in the constructor:
newObject.output.setTypeAtLeast(newObject.input);
newObject.output.setTypeAtLeast(newObject.factor);
Note that we set the type constraint to the cloned port not to the original port. Thus:
// WRONG: use the cloned ports, not the ports of the cloned object!
newObject.output.setTypeAtLeast(input);
newObject.output.setTypeAtLeast(factor);
is wrong, set the constraint to the cloned port.
Note that at no time during cloning is any constructor invoked, so it is necessary to repeat in the
clone() method any initialization in the constructor. For example, the clone() method in the Expression
actor sets the values of a few private variables:
newObject._iterationCount = 1;
newObject._time = (Variable)newObject.getAttribute("time");
newObject._iteration =
(Variable)newObject.getAttribute("iteration");
--end---
Colin knows all about cloning, I'm including the above text because
it is a tricky part of writing actors and worth reviewing.
Myself, I get confused about which type methods must be in the clone(Workspace)
method.
_Christopher
On 5/24/10 3:59 PM, Colin Enticott wrote:
> Hi Christopher,
>
> Thank you for your reply. Yes, it is an odd one. I've had the problem
> for a little while, but end up doing things other ways to make it
> work.
>
> The problem occurs with my director. What type of small test case
> would you like? I could write a small actor that clones itself on a PN
> composite? I guess the multi instance composite would do the trick. I
> would only need some parameter dependencies for it.
>
> Regards,
> Colin
>
> On 25 May 2010 04:02, Christopher Brooks<cxh at eecs.berkeley.edu> wrote:
>> Hi Colin,
>> Yes, this looks like some sort of cloning problem. I'd need a small
>> test case to do much more analysis on this. As you say, it could be
>> a problem with _parseTreeEvaluator not being properly reset.
>> Perhaps Variable.clone() should set _parseTreeEvaluator to null?
>>
>> It is odd to me that your error message is for $h instead of $a or $j.
>> I wonder why that is? The error does seem to be a scoping issue.
>>
>> _Christopher
>>
>> On 5/24/10 12:42 AM, Colin Enticott wrote:
>>>
>>> Hi Christopher,
>>>
>>> I'm having trouble again when I use the clone(Workspace) method again
>>> on an active workflow. This time I am getting an exception when
>>> evaluating an expression. The actor has an expression:
>>> $a $b $c $d $e $f $g $h $i $j
>>>
>>> The Parameter "a" refers to parameter "A" and likewise for the rest
>>> (yes, this is a test workflow :-) ).
>>>
>>> After the following code to clone the actor into a new workspace, I
>>> get the following exception:
>>>
>>> _actor = (Actor)((Entity)_parentActor).clone(_workspace);
>>> _actor.setName(name);
>>> ((ComponentEntity)_actor).setContainer(tca);
>>>
>>>
>>> ptolemy.kernel.util.IllegalActionException: Error evaluating
>>> expression: $a $b $c $d $e $f $g $h $i $j
>>> in .ParameterDepend.String Constant_C2_TCA.String Constant_C2.value
>>> Because:
>>> The ID h is undefined.
>>> with tag colour {sequenceID=2, batchSize=1024, parameters={A = 1, B
>>> = 1, C = 1, D = 1, E = 1, F = 1, G = 1, H = 1, I = 2, J = 1},
>>> hashcode=-1226196380}
>>> in .ParameterDepend.String Constant_C2_TCA.String Constant_C2
>>> at ptolemy.data.expr.Variable.validate(Variable.java:1449)
>>> at
>>> ptolemy.kernel.util.NamedObj._validateSettables(NamedObj.java:2658)
>>> at ptolemy.kernel.Entity._validateSettables(Entity.java:651)
>>> at
>>> ptolemy.kernel.util.NamedObj.validateSettables(NamedObj.java:1972)
>>> at
>>> org.monash.nimrod.NimrodDirector.NimrodProcessThread._updateParameters(NimrodProcessThread.java:692)
>>> at
>>> org.monash.nimrod.NimrodDirector.NimrodProcessThread.run(NimrodProcessThread.java:389)
>>>
>>>
>>> But if I change the code into this slower, more java memory hungry
>>> version, it works:
>>> String s = ((Entity)_parentActor).exportMoML(name);
>>> MoMLChangeRequest l = new MoMLChangeRequest(this, tca, s);
>>> l.execute();
>>> _actor = (Actor)tca.getEntity(name);
>>>
>>> Looking at the code for th Variable class, it is hard for me to tell
>>> what isn't being updated. The best guess I have is I can't see
>>> _parseTreeEvaluator being reset in the clone(Workspace) method. The
>>> _parseTreeEvaluator holds onto state including a "ParserScope". Could
>>> this be the problem?
>>>
>>> Or is it something else I am doing?
>>>
>>> Thanks,
>>> Colin
>>>
>>>
>>> On 4 January 2010 05:04, Christopher Brooks<cxh at eecs.berkeley.edu> wrote:
>>>>
>>>> Hi Colin,
>>>> Thanks, I wrote a test that illustrates this problem and then
>>>> fixed it.
>>>>
>>>> The test is
>>>>
>>>> test NamedObj-8.1.0 {clone should not have _debugListener set} {
>>>> set n [java::new ptolemy.kernel.util.Workspace "N"]
>>>> set a [java::new ptolemy.kernel.util.NamedObj $n "A" ]
>>>> set listener [java::new ptolemy.kernel.util.RecorderListener]
>>>> $a addDebugListener $listener
>>>> set b [java::cast ptolemy.kernel.util.NamedObj [$a clone]]
>>>> set listenerB [java::new ptolemy.kernel.util.RecorderListener]
>>>> $b addDebugListener $listenerB
>>>> $b setName B
>>>> $b clone
>>>> list [$listener getMessages] \
>>>> [$listenerB getMessages]
>>>> } {{Cloned .A into workspace: N
>>>> } {Changed name from .A to .B
>>>> Cloned .B into workspace: N
>>>> }}
>>>>
>>>>
>>>> Before the fix, the result was:
>>>>
>>>> {Cloned .A into workspace: N
>>>> Changed name from .A to .B
>>>> Cloned .B into workspace: N
>>>> } {Changed name from .A to .B
>>>> Cloned .B into workspace: N
>>>> }
>>>>
>>>> which indicates that the listener to A is getting messages intended for
>>>> B.
>>>>
>>>> _Christopher
>>>>
>>>> On 12/28/09 9:15 PM, Colin Enticott wrote:
>>>>>
>>>>> Hi Christopher,
>>>>>
>>>>> Along the same lines as this old bug, it seems _debugListeners is not
>>>>> being nulled on the new object after clone(Workspace) is called.
>>>>>
>>>>> It's funny I noticed problems with debugging after two years working
>>>>> with Kepler. It's obvious I don't debug my code enough ;-)
>>>>>
>>>>> Oh, I'm still working with Kepler-1.0.0. If this has been fixed since,
>>>>> I apologise.
>>>>>
>>>>> Kind regards,
>>>>> Colin
>>>>>
>>>>> 2008/2/23 Colin Enticott<Colin.Enticott at infotech.monash.edu.au>:
>>>>>>
>>>>>> Thanks Dan and Christoper. Works a treat.
>>>>>>
>>>>>> Colin
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: cxh at eecs.berkeley.edu [mailto:cxh at eecs.berkeley.edu]
>>>>>> Sent: Saturday, 23 February 2008 6:14 AM
>>>>>> To: Daniel Crawl
>>>>>> Cc: Colin Enticott; Kepler-Dev
>>>>>> Subject: Re: [kepler-dev] IOPortEvent bug
>>>>>>
>>>>>> Done!
>>>>>>
>>>>>>
>>>>>> cxh at carson 192% cvs diff -D yesterday IOPort.java
>>>>>> Index: IOPort.java
>>>>>> ===================================================================
>>>>>> RCS file: /home/cvs/ptII/ptolemy/actor/IOPort.java,v
>>>>>> retrieving revision 1.260
>>>>>> retrieving revision 1.261
>>>>>> diff -r1.260 -r1.261
>>>>>> 129c129
>>>>>> < @version $Id: IOPort.java,v 1.260 2008/02/19 18:25:47 cxh Exp $
>>>>>> ---
>>>>>>>
>>>>>>> @version $Id: IOPort.java,v 1.261 2008/02/22 19:11:58 cxh Exp $
>>>>>>
>>>>>> 420a421,424
>>>>>>>
>>>>>>> newObject._hasPortEventListeners = false;
>>>>>>> newObject._portEventListeners = null;
>>>>>>>
>>>>>> cxh at carson 193%
>>>>>>
>>>>>> On my todo list is to write some tests for the IOPortListener
>>>>>> code.
>>>>>>
>>>>>> _Christopher
>>>>>>
>>>>>> --------
>>>>>>
>>>>>>
>>>>>> Hi Colin,
>>>>>>
>>>>>> The cloned IOPort's listener list should be set to null. Since
>>>>>> Kepler CVS no longer has a separate copy of IOPort.java, could
>>>>>> someone make this change in ptII CVS?
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> --dan
>>>>>>
>>>>>> Colin Enticott wrote:
>>>>>> > Hi all,
>>>>>> >
>>>>>> > I was having trouble with my code and I have narrowing it down
>>>>>> to
>>>>>> > IOPortEvents feature. There appears to be a bug in the IOPort
>>>>>> class
>>>>>> with
>>>>>> > IOPortEvent feature and cloning. It looks like if there has
>>>>>> been
>>>>>> an
>>>>>> > IOPortEventListener added, when it is cloned, all the clones
>>>>>> will
>>>>>> have
>>>>>> a
>>>>>> > reference to the same listener list. I guess the list itself
>>>>>> should
>>>>>> be
>>>>>> > cloned or the clone's list reference should be set back to
>>>>>> null.
>>>>>> >
>>>>>> > It is possible if someone could have a look at this?
>>>>>> >
>>>>>> > Thanks,
>>>>>> > Colin
>>>>>> >
>>>>>> > _______________________________________________
>>>>>> > Kepler-dev mailing list
>>>>>> > Kepler-dev at ecoinformatics.org
>>>>>> >
>>>>>>
>>>>>> http://mercury.nceas.ucsb.edu/ecoinformatics/mailman/listinfo/kepler-dev
>>>>>> >
>>>>>>
>>>>>> _______________________________________________
>>>>>> Kepler-dev mailing list
>>>>>> Kepler-dev at ecoinformatics.org
>>>>>>
>>>>>>
>>>>>> http://mercury.nceas.ucsb.edu/ecoinformatics/mailman/listinfo/kepler-dev
>>>>>> --------
>>>>>>
>>>>>> _______________________________________________
>>>>>> Kepler-dev mailing list
>>>>>> Kepler-dev at ecoinformatics.org
>>>>>>
>>>>>> http://mercury.nceas.ucsb.edu/ecoinformatics/mailman/listinfo/kepler-dev
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>> --
>>>> Christopher Brooks, PMP University of California
>>>> CHESS Executive Director US Mail: 337 Cory Hall
>>>> Programmer/Analyst CHESS/Ptolemy/Trust Berkeley, CA 94720-1774
>>>> ph: 510.643.9841 fax:510.642.2718 (Office: 545Q Cory)
>>>> home: (F-Tu) 707.665.0131 cell: 707.332.0670
>>>>
>>>
>>>
>>>
>>
>> --
>> Christopher Brooks, PMP University of California
>> CHESS Executive Director US Mail: 337 Cory Hall
>> Programmer/Analyst CHESS/Ptolemy/Trust Berkeley, CA 94720-1774
>> ph: 510.643.9841 fax:510.642.2718 (Office: 545Q Cory)
>> home: (F-Tu) 707.665.0131 cell: 707.332.0670
>>
>
>
>
--
Christopher Brooks, PMP University of California
CHESS Executive Director US Mail: 337 Cory Hall
Programmer/Analyst CHESS/Ptolemy/Trust Berkeley, CA 94720-1774
ph: 510.643.9841 fax:510.642.2718 (Office: 545Q Cory)
home: (F-Tu) 707.665.0131 cell: 707.332.0670
More information about the Kepler-dev
mailing list