Kepler: Issueshttps://projects.ecoinformatics.org/ecoinfo/https://projects.ecoinformatics.org/ecoinfo/ecoinfo/favicon.ico?14691340362011-11-18T00:50:15ZEcoinformatics Redmine
Redmine Bug #5546 (Resolved): SDF director iterations parameter default of 0 is unfriendlyhttps://projects.ecoinformatics.org/ecoinfo/issues/55462011-11-18T00:50:15ZDerik Barseghianbarseghian@nceas.ucsb.edu
<p>I know this was debated/changed at least once in the past, but I can't remember many of the details...at one point Aaron change the default to 1, but I think did so in a way that affected existing models. <br />If we could change SDF's iterations default to 1 without affecting existing models, we'd simply the common use case and remove an extremely common stumbling block for new users.</p> Bug #4872 (Resolved): listen to director throws errorhttps://projects.ecoinformatics.org/ecoinfo/issues/48722010-03-05T22:14:10ZOliver Soongsoong@nceas.ucsb.edu
<p>Add an SDF Director and the Tools->Listen to Director. An error pops up on the console:</p>
<p>Error creating action for class: org.kepler.gui.RunWithFeedbackChkBoxAction : null<br />Error creating action for class: org.kepler.gui.RunWithFeedbackChkBoxAction</p>
<p>I'm not sure if this causes problems or not.</p> Bug #4256 (Resolved): Change to AbstractReceiver prevents putArray() from being calledhttps://projects.ecoinformatics.org/ecoinfo/issues/42562009-07-22T17:18:37ZChristopher Brookscxh@eecs.berkeley.edu
<p>Here's a bug from 5/13.</p>
<p>I agree that the problem is that putArray()<br />is no longer being called, which is being overridden in the derived<br />classes. domains/modal/kernel/FSMReceiver.java also calls putArray(),<br />so this is relevant.</p>
<p>I think the right solution is to modify ArrayToSequence and remove<br />the change to to AbstractReceiver.java</p>
<p>I'd like to see this fixed before we roll out Ptolemy II 8.0.beta.</p>
<p>On 5/13, Bert Rodiers wrote:</p>
<blockquote>
<p>Hello Christopher,</p>
<p>Another issue with this change is that putArray is no longer called, which<br />could be overridden by derived classes. Now this functionality is bypassed.<br />For example:<br />CIReceiver has this implementation:<br />public synchronized void putArray(Token[] tokenArray, int<br />numberOfTokens)<br />throws NoRoomException {<br />for (int i = 0; i < numberOfTokens; i++) {<br />_tokens.add(tokenArray[i]);<br />}</p>
<p>_notify();<br />}</p>
<p>FSMReceiver has this implementation:<br />public void putArray(Token[] tokenArray, int numberOfTokens)<br />throws NoRoomException, IllegalActionException {<br />if (numberOfTokens != 1 || tokenArray.length < 1) {<br />throw new IllegalActionException(getContainer(), "Receiver<br />cannot accept more than one token.");<br />}<br />put(tokenArray<sup><a href="#fn0">0</a></sup>);<br />}</p>
<p>and so on...</p>
<p>Regards,<br />Bert</p>
</blockquote>
<p>Edward wrote:</p>
<blockquote>
<p>If the only difference is the order the loops,<br />I think we are OK with the change in AbstractReceiver.</p>
<p>Edward</p>
</blockquote>
<blockquote>
<p>2009/5/13 Christopher Brooks <<a class="email" href="mailto:cxh@eecs.berkeley.edu">cxh@eecs.berkeley.edu</a>></p>
<blockquote>
<p>Hi Dan,<br />I folded in your change.</p>
<p>One thing that has me concerned is that if the model mutates<br />between when we check the containers and when we do<br />the conversion, we could have problems.</p>
<p>For example, when I run<br />$PTII/bin/vergil ~/ptII/ptolemy/domains/pn/kernel/test/auto/block.xml</p>
<p>putArrayToAll() is called:</p>
<p>ptolemy.actor.AbstractReceiver.putArrayToAll(AbstractReceiver.java:297)<br />at ptolemy.actor.IOPort.send(IOPort.java:2728)<br />at ptolemy.actor.TypedIOPort.send(TypedIOPort.java:524)<br />at ptolemy.domains.sdf.lib.ArrayToSequence.fire(ArrayToSequence.java:176)<br />at ptolemy.actor.process.ProcessThread.run(ProcessThread.java:217)</p>
<p>I don't see where a lock is held that would prevent the model<br />from mutating between when we check the containers and when<br />we use them.</p>
<p>Interestingly, the original code had a similar problem, so<br />this is not a new issue.</p>
<p>I'm probably overlooking something . . .</p>
<p>To address Edward's concern, while I feel that this is a fundamental<br />change, we do have a trivial example that needs the change and<br />we don't have a counter example that is broken by the change.</p>
<p>The difference between what we had before and what we have now isr<br />the order of the loops. Previously, we looped through<br />the receivers in the outer loop and the tokens in the inner loop.<br />Now we loop through the tokens in the outer loop and the<br />receivers in the inner loop.</p>
<p>Why should this make a difference?</p>
<p>_Christopher</p>
<p>Hi Bert and Christopher,</p>
<blockquote>
<p>Thanks for clarifying the performance problem, and adding<br />my update and test case.</p>
<p>What do you think of the following putArrayToAll? I think<br />this is what Bert suggested; it does not make extra calls<br />to getContainer.</p>
<p>public void putArrayToAll(Token[] tokens, int numberOfTokens,<br />Receiver[] receivers) throws NoRoomException,<br />IllegalActionException {<br />if (numberOfTokens > tokens.length) {<br />IOPort container = getContainer();<br />throw new IllegalActionException(container,<br />"Not enough tokens supplied.");<br />}</p>
<p>// Cache the containers for each receiver to minimize<br />// the number of calls to getContainer.<br />IOPort[] containers = new IOPort[receivers.length];<br />for (int j = 0; j < receivers.length; j++) {<br />containers[j] = receivers[j].getContainer();<br />}</p>
<p>// Loop through the tokens on the outer loop and<br />// the receivers on the inner loop. See<br />// pn/kernel/test/block.xml for a test case<br />// (Bug fix proposed by Daniel Crawl.)<br />for(int i = 0; i < numberOfTokens; i++) {<br />for (int j = 0; j < receivers.length; j++ ) {<br />if (containers[j] == null) {<br />receivers[j].put(tokens[i]);<br />} else {<br />receivers[j].put(containers[j].convert(tokens[i]));<br />}<br />}<br />}<br />}</p>
<p>Thanks,</p>
<p>--dan</p>
</blockquote>
</blockquote></blockquote>
<p>On Wed May 13 10:12:27 PDT 2009<br />Sean Riddle wrote:</p>
<blockquote>
<p>I agree that the correct approach would be to modify ArrayToSequence. <br />I can't remember quite why, but I also needed this behavior from <br />ArrayToSequence at some point, and the change is only one or two <br />lines, plus you're guaranteed that this won't affect other parts of <br />the system unexpectedly.</p>
<p>- Sean</p>
</blockquote>
<p>On Wed May 13 08:32:39 PDT 2009</p>
<blockquote><blockquote>
<p>Edward A. Lee wrote:</p>
<blockquote>
<p>I see.</p>
<p>It seems to me that the "right" fix is to modify ArrayToSequence,<br />not to modify the mechanism in AbstractReceiver. The reason for<br />the mechanism in AbstractReceiver is to support bulk transfers<br />efficiently. I'm not sure where else this is used, but it does<br />seem that preserving this capability would be useful.</p>
<p>I don't feel that strongly about it though...</p>
<p>Edward</p>
<p>Jianwu Wang wrote:</p>
<blockquote>
<p>Hi Edward,</p>
<p>Dan and I found the problem from a real world workflow. The workflow<br />includes about 50 job files, their execution time varies from 1 hours to 5<br />hours. The jobs can be submitted by firing JobSubmitter actor 50 times with<br />corresponding inputs, and their status can be checked by JobStatus actor in<br />a loop. I hope once one job is finished, the following actors related to the<br />data of this job can be processed without waiting for other jobs. So that we<br />can enable pipeline parallel. The workflow has the structure similar with<br />the attached test case. With default PN director configuration,<br />ArrayToSequence actor tries to send all array data to one next actor before<br />sending data to another connected actor. But the Equals actor need the data<br />from the two Expressions to be fired. So the Equals actor will increase its<br />capacity and not be fired since there is still running actor (job actors).<br />The workflow has to wait all jobs are finished before processing the<br />following actors, which is very inefficient for my case.</p>
<p>With Dan's modification, the ArrayToSequence actor can transfer the<br />first token of the array to all the following actors before transfer the<br />second token. Then the Equals actor can be fired timely and do not need to<br />increase capacity. So the same workflow can realize real pipeline parallel,<br />namely the following actors can be fired immediately with corresponding data<br />once one job is finished.</p>
<p>Best wishes</p>
<p>Sincerely yours</p>
<p>Jianwu Wang<br /><a class="email" href="mailto:jianwu@sdsc.edu">jianwu@sdsc.edu</a><br /><a class="external" href="http://users.sdsc.edu/~jianwu/">http://users.sdsc.edu/~jianwu/</a> <<a class="external" href="http://users.sdsc.edu/%7Ejianwu/">http://users.sdsc.edu/%7Ejianwu/</a>></p>
<p>Scientific Workflow Automation Technologies (SWAT) Laboratory<br />San Diego Supercomputer Center University of California, San Diego<br />San Diego, CA, U.S.A.</p>
<p>Edward A. Lee wrote:</p>
<blockquote>
<p>I don't really understand the problem being solved here.<br />The deadlock is a consequence of setting the maximum queue capacity<br />to one. Why would you want to do that?</p>
<p>Edward</p>
<p>Daniel Crawl wrote:</p>
<blockquote>
<p>Hi Edward,</p>
<p>Attached is the test case. I set the max queue capacity to one so<br />that an exception is thrown instead of an artificial deadlock<br />occurring. With my change, the exception does not occur and the<br />model finishes.</p>
<p>Is there a test case demonstrating the performance problem? In<br />both versions, put is called (with a single token) the same number<br />of times, so it's not clear how my change could hurt efficiency.</p>
<p>Thanks,</p>
<p>--dan</p>
<p>Edward A. Lee wrote:</p>
<blockquote>
<p>Dan,</p>
<p>Are you sure the deadlock is artificial?<br />I would like to see the test case. Maybe the model isn't using<br />the right actors?</p>
<p>The point of the methods you changed was to improve<br />efficiency, and sending tokens one at a time nullifies<br />that point. There is really not point in even having these<br />methods, I think.</p>
<p>Edward</p>
<p>Daniel Crawl wrote:</p>
<blockquote>
<p>Hi Christopher,</p>
<p>I made this update to prevent unnecessary artificial deadlocks<br />in PN under certain circumstances. I can add a test case that<br />demonstrates the problem.</p>
<p>If the convert is performed, is the update ok? Note that no<br />tests failed in ptolemy/actor/test/ due to this change...<br />Since calling convert is essential, I can also add a test case<br />for this.</p>
<p>There were effectively two nested loops before, so I do not see<br />how this change could degrade performance. If it is measurably<br />different, it is improved since the outer loop no longer calls<br />a method.</p>
<p>Thanks,</p>
<p>--dan</p>
<p>Christopher Brooks wrote:</p>
<blockquote>
<p>Yep, I went ahead and reverted the change.</p>
<p>_Christopher</p>
<p>Edward A. Lee wrote:</p>
<blockquote>
<p>The call to convert is essential.</p>
<p>Without it, we'll get some very esoteric and difficult to track<br />type system bugs. A likely manifestation is that actors will<br />start throwing ClassCastException because they have declared<br />an input to be double, so they cast incoming tokens to DoubleToken.<br />Without the call to convert(), they may get, say, an IntToken.<br />This will be a very unfriendly error...</p>
<p>Edward</p>
<p>Christopher Brooks wrote:</p>
<blockquote>
<p>Hi Daniel,<br />I'm concerned that this is a performance hit because we<br />have two nested loops. Can you tell me more about why this<br />change is necessary? Do you have a test case that illustrates<br />the bug? Without a test case, it is not likely that the fix will<br />persist, though the comment should help.</p>
The entire method is:<br />/** Put a sequence of tokens to all receivers in the specified<br />array.
<ul>
<li> Implementers will assume that all such receivers</li>
<li> are of the same class.</li>
<li> @param tokens The sequence of token to put.</li>
<li> @param numberOfTokens The number of tokens to put (the<br />array might</li>
<li> be longer).</li>
<li> @param receivers The receivers.</li>
<li> @exception NoRoomException If there is no room for the<br />token.</li>
<li> @exception IllegalActionException If the token is not<br />acceptable</li>
<li> to one of the ports (e.g., wrong type), or if the tokens<br />array</li>
<li> does not have at least the specified number of tokens.<br />*/<br />public void putArrayToAll(Token[] tokens, int numberOfTokens,<br />Receiver[] receivers) throws NoRoomException,<br />IllegalActionException {<br />if (numberOfTokens > tokens.length) {<br />IOPort container = getContainer();<br />throw new IllegalActionException(container,<br />"Not enough tokens supplied.");<br />}</li>
</ul>
<p>// Put a single token at a time for each receiver instead<br />of<br />// putting the entire array. In the latter case, we may<br />block<br />// on a receiver while other receiver(s) starve.<br />for(int i = 0; i < numberOfTokens; i++) {<br />for (int j = 0; j < receivers.length; j++ ) {<br />receivers[j].put(tokens[i]);<br />}<br />}<br />}</p>
<p>I do see how this could be a problem with blocking though.</p>
<p>Your change is to call put() instead of putArray().<br />AbstractReceiver.putArray() looks like:</p>
<p>public void putArray(Token[] tokenArray, int numberOfTokens)<br />throws NoRoomException, IllegalActionException {<br />IOPort container = getContainer();</p>
<p>// If there is no container, then perform no conversion.<br />if (container == null) {<br />for (int i = 0; i < numberOfTokens; i++) {<br />put(tokenArray[i]);<br />}<br />} else {<br />for (int i = 0; i < numberOfTokens; i++) {<br />put(container.convert(tokenArray[i]));<br />}<br />}<br />}</p>
<p>It looks like your change is ok when the container is null, but<br />in the AbstractReceiver base class it does not handle the call<br />to convert()? I'm not sure if this is important or not.</p>
<p>I'm fairly certain that putArrayToAll() will be called when we<br />call<br />IOPort.broadcast.</p>
<p>What do you think?</p>
<p>_Christopher</p>
<p>Daniel Crawl wrote:</p>
<blockquote>
<p>Author: crawl<br />Date: 2009-05-06 14:45:46 -0700 (Wed, 06 May 2009)<br />New Revision: 53516</p>
<p>Modified:<br />trunk/ptolemy/actor/AbstractReceiver.java<br />Log:<br />Put a single token at a time for each receiver in<br />putArrayToAll().</p>
<p>Modified: trunk/ptolemy/actor/AbstractReceiver.java
===================================================================</p>
<p>--- trunk/ptolemy/actor/AbstractReceiver.java 2009-05-06<br />21:13:26 UTC (rev 53515)<br />+<ins>+ trunk/ptolemy/actor/AbstractReceiver.java 2009-05-06<br />21:45:46 UTC (rev 53516)<br /><code>@ -300,8 +300,13 </code>@<br />"Not enough tokens supplied.");<br />}<br />- for (int j = 0; j < receivers.length; j</ins>+) {<br />- receivers[j].putArray(tokens, numberOfTokens);<br />+ // Put a single token at a time for each receiver<br />instead of<br />+ // putting the entire array. In the latter case, we may<br />block<br />+ // on a receiver while other receiver(s) starve.<br />+ for(int i = 0; i < numberOfTokens; i++) {<br />+ for (int j = 0; j < receivers.length; j++ ) {<br />+ receivers[j].put(tokens[i]);<br />+ }<br />}<br />}</p>
<p>_<em><i></em></i>__<em>_</em>___________________________________<br />Ptexternal-cvs mailing list<br /><a class="email" href="mailto:Ptexternal-cvs@chess.eecs.berkeley.edu">Ptexternal-cvs@chess.eecs.berkeley.edu</a><br /><a class="external" href="http://chess.eecs.berkeley.edu/ptexternal/listinfo/ptexternal-cvs">http://chess.eecs.berkeley.edu/ptexternal/listinfo/ptexternal-cvs</a></p>
</blockquote>
</blockquote></blockquote></blockquote>
<p>_<em><i></em></i>__<em>_</em>___________________________________<br />Ptolemy maillist - <a class="email" href="mailto:Ptolemy@chess.eecs.berkeley.edu">Ptolemy@chess.eecs.berkeley.edu</a><br /><a class="external" href="http://chess.eecs.berkeley.edu/ptolemy/listinfo/ptolemy">http://chess.eecs.berkeley.edu/ptolemy/listinfo/ptolemy</a></p>
</blockquote></blockquote>
<hr />
</blockquote>
<p>Kepler-dev mailing list<br /><a class="email" href="mailto:Kepler-dev@kepler-project.org">Kepler-dev@kepler-project.org</a><br /><a class="external" href="http://mercury.nceas.ucsb.edu/kepler/mailman/listinfo/kepler-dev">http://mercury.nceas.ucsb.edu/kepler/mailman/listinfo/kepler-dev</a></p>
</blockquote>
<p>_<em><i></em></i>__<em>_</em>___________________________________<br />Ptolemy maillist - <a class="email" href="mailto:Ptolemy@chess.eecs.berkeley.edu">Ptolemy@chess.eecs.berkeley.edu</a><br /><a class="external" href="http://chess.eecs.berkeley.edu/ptolemy/listinfo/ptolemy">http://chess.eecs.berkeley.edu/ptolemy/listinfo/ptolemy</a></p>
</blockquote></blockquote>
<p>--<br />Christopher Brooks (cxh at eecs berkeley edu) University of California<br />CHESS Executive Director US Mail: 337 Cory Hall<br />Programmer/Analyst CHESS/Ptolemy/Trust Berkeley, CA 94720-1774<br />ph: 510.643.9841 fax:510.642.2718 (Office: 545Q Cory)<br />home: (F-Tu) 707.665.0131 (W-F) 510.655.5480<br />_<em><i></em></i>__<em>_</em>___________________________________<br />Ptolemy maillist - <a class="email" href="mailto:Ptolemy@chess.eecs.berkeley.edu">Ptolemy@chess.eecs.berkeley.edu</a><br /><a class="external" href="http://chess.eecs.berkeley.edu/ptolemy/listinfo/ptolemy">http://chess.eecs.berkeley.edu/ptolemy/listinfo/ptolemy</a></p>
</blockquote>
</blockquote> Bug #2514 (Resolved): CT Director parameter list for ODESolver is incompletehttps://projects.ecoinformatics.org/ecoinfo/issues/25142006-08-15T20:09:25ZDan Higginshiggins@nceas.ucsb.edu
<p>The default value of the ODESolver parameter is "ExplicitRK45Solver" but that value does not appear in the drop down list of choices. Thus, one changed, it cannot be reset!</p>