Kepler

This is a duplicate of bug #3774:
"Hello World demo causes UI to become unresponsive and require restart"
Rather than setting the iterations to 1, we should fix the underlying bug.

Edward and I have talked about this bug recently. I'll see what can be done.

Reopening, I don't think this is a duplicate, though it's clearly related.

bug#3774 seems to be more about making the Stop button work more promptly during the run of an infinite workflow. That will be great, but even so, I don't think a first time user anticipates their first workflow using SDF to run infinitely. With this bug I'm suggesting we change the default of SDF iterations to 1 -- I've heard it as a complaint from a few users.

What do people think of this? Granted it would come at the cost of having to update our docs to remove that step.

Reopening this is fine with me, though I think that changing the iteration
from 0 (run forever) to 1 is a mistake because 1 iteration producing one data point is not usually very useful. There was a bug where the plotter would not even plot one data point.

If you want to go ahead and do this, then probably the easiest way to do
this is to modify
kepler/directors/resources/kar/CoreDirectors/SDFDirector.xml
and add
<property name="iterations" class="ptolemy.data.expr.Parameter" value="1"/>

On 8/4/12 10:53 AM, Edward A. Lee wrote:

Jennifer suggests changing the default iterations count of the SDF
director from 0 to 1. I think this is a good idea, but we should
then change all the demos that use the default. Any idea how
to find them? These would be demos with an SDFDirector that
do not set the iterations parameter.

I hacked up an awk script that attempts to insert the iterations parameter
if it is not set in SDFDirector or any of the derived classes.

The script is at $PTII/ptolemy/domains/sdf/kernel/fixSDFIterations

Basically, it looks for SDFDirector or its subclasses and the inserts the
iterations parameter if it is not already set.

I ran the tests and while there are a number of failures, it is not looking
so bad.

Shall I check in the changed .xml files?

If we make the default iterations be 1, then the Ptolemy and Kepler
actor and printed documentation will need to be updated.

One downside is that using an SDFDirector in an inner composite will
typically require changing the iterations.

Not sure I understand -- which xml files get changed? I wouldn't think we'd want to change existing demo momls, they're probably set/not set intentionally.

If SDFDirector is used inside a non-toplevel composite
(aka an Opaque Composite), then the number of iterations
is typically the default, which is 0.

If we change the default, then the behavior might change.

Edward may have a better explanation.

Ok I think I understand, I forgot that it's not always explicitly set to something.
So we could presumably fix all the demos by explicitly setting it, if it's not, to the current default of 0, to maintain current behavior. But it seems like we'll also need to add a filter to do the same thing so all workflows we don't control get updated on opening.

I'm appending the thread in pt-dev about this.
I made the fix (call isEmbedded()).

Derik asked about creating a filter that would set iterations to 0.
We don't have such a filter, but I believe that the implementation below
will do the trick. For preexisting models that do not have iterations set,
then the default will be "AUTO", which will provide the correct functionality.

Also, I created ptolemy/domains/sdf/kernel/fixSDFIterations, which is a script
that forces iterations to be set to 0. I'm not sure if it is needed.

I'll open a documentation bug to update the docs for this change.

This bug is almost ready to be closed.

On 8/10/12 7:00 PM, Edward A. Lee wrote:

One minor point: The director should call isEmbedded()
instead of seeing whether the container's container is null.
The reason for this is RunCompositeActor, where the container's
container is not null, but you still want the model to behave
as if it were at the top level...

I admit that this design is not ideal. Any ideas for a better
way?

Edward

On 8/10/12 4:33 PM, Christopher Brooks wrote:

Ok, I implemented your suggested changes.

The SDF Constructor eventually calls:

// AUTO and UNBOUNDED are used to set the value of iterations,
// see the getIterations() method.

Parameter AUTO = new Parameter(this, "AUTO");
AUTO.setToken(_auto);
AUTO.setVisibility(Settable.EXPERT);

Parameter UNBOUNDED = new Parameter(this, "UNBOUNDED");
UNBOUNDED.setToken(IntToken.ZERO);
UNBOUNDED.setVisibility(Settable.EXPERT);

iterations = new Parameter(this, "iterations");
iterations.setTypeEquals(BaseType.INT);
iterations.addChoice("AUTO");
iterations.addChoice("UNBOUNDED");
iterations.setExpression("AUTO");

I added the following method that determines how iterations are handled:

/** Return the number of iterations. *

• <p>The number of iterations returned depends on the value of
• the <i>iterations</i> parameter and whether the container
• of the director is at the top level.
• See the {@link #interations} documentation for details.</p> *
• <p>Code that uses SDFDirector should call getIterations()
• instead of directly referring to the value of the
• <i>iterations</i> parameter. *
• @return the number of iterations
• @exception If thrown while getting the value of the
• iterations parameter.
  */
  public int getIterations() throws IllegalActionException {
  // See "SDF director iterations parameter default of 0 is unfriendly"
  // http://bugzilla.ecoinformatics.org/show_bug.cgi?id=5546
  int iterationsValue = ((IntToken) (iterations.getToken())).intValue();
  if (iterationsValue > 0) {
  return iterationsValue;
  }
  NamedObj container = getContainer();
  if (container != null
  && container.getContainer() == null) {
  // The container of this director is at the toplevel
  if (iterations.equals(_auto)) {
  return 1;
  }
  }
  return 0;
  }

It will take a few days for any bugs to settle out of the nightly build.

_Christopher

On 8/9/12 2:12 PM, Edward A. Lee wrote:

How about if we define a local variable called AUTO
(that has value -1), and document that AUTO means what
you say below. This seems like a good solution...
We should also define a variable called UNBOUNDED
that has value 0, with the current meaning of 0.
Then we should enhance the parameter with
addChoice("AUTO") and addChoise("UNBOUNDED").

Edward

On 8/9/12 9:06 AM, Christopher Brooks wrote:

In the perfect world, if the SDFDirector was at the top level,
the initial number of iterations would be 1.

If the SDFDirector was not at the top level,
the initial number of iterations would be 0.

Perhaps we should set the initial value to -1, which would have
the above behavior?

I dunno.

Right now, the initial default for iterations is 1.

The test failures are not that bad, so let's leave it be for awhile.

The bug is at
http://bugzilla.ecoinformatics.org/show_bug.cgi?id=5546

_Christopher

On 8/9/12 8:39 AM, Edward A. Lee wrote:

Oops... I forgot about the non-top-level problem.
Setting it to 1 by default might create major problems...
People will be very puzzled ...
Maybe we need to be smarter about this...

Edward

On 8/9/12 8:34 AM, Christopher Brooks wrote:

Jennifer,
Also, the SDFDirector iterations parameter now defaults to 1.
Previously, it defaulted to 0, which meant "run forever".

Edward, my understanding is that iterations should usually be set to 0
when SDFDirector is not at the top level. Is this the case?
Should we have some text about this?

Closing.
There is a separate bug #5677 for documentation changes.

Original Bugzilla ID was 5546