RE: ISSUE-826: comments from Gavin Kistner [SCXML-LC-Comments]

Gavin,
  Thank you for the detailed comments.  I've put responses to some of them below, set off by '>>'.  Others will require more thought.  

-----Original Message-----
From: Voice Browser Working Group Issue Tracker [mailto:sysbot+tracker@w3.org] 
Sent: Friday, January 25, 2013 9:11 AM
To: w3c-voice-wg@w3.org; www-voice@w3.org
Subject: ISSUE-826: comments from Gavin Kistner [SCXML-LC-Comments]

ISSUE-826: comments from Gavin Kistner [SCXML-LC-Comments]

https://www.w3.org/Voice/Group/track/issues/826


Raised by: James Barnett
On product: SCXML-LC-Comments

I have created a 90%-working SCXML interpreter written in the Ruby programming language. Following are notes I kept during implementation about confusions/bugs/issues I ran into. Hopefully some of these can be used to improve a future draft of the standard. I couldn't decide if I should start one thread per item or not, so I took the lazy route and have included them all numbered below. Please feel free to fork the discussion into more targeted points with changed subjects as appropriate.

Note that several of the items below are not just recommendations for change, but also personal requests for clarification. I would greatly appreciate direct responses for items 8 and 12-16 below.

1. The Predicates section of Appendix A and some prose mentions "entryOrder", but the only location where this is used in the pseudo-code it is used as "enterOrder".
>> I'll fix this.  It should be "entryOrder" everywhere

2. Appendix A uses isDescendant(a,b) in many locations, but never defines: is it testing that a is a descendant of b, or that b is a descendant of a? Without specifying any implementation, a note would be helpful describing this function directly (and any other function where the spec authors assumed that the function/procedure title was self-evident.)
>> It tests whether a is descended from b.  I will add a clarification.

3. Prose in various sections of Appendix A uses the word "proper" as in "proper ancestors" and "proper descendants", but the meaning of this phrase is never made clear. What is an improper ancestor? :) Why is this word used at all? (My eventual conclusion was that it meant that the starting element cannot be considered part of its "ancestors" or "descendants", but I don't know if this is correct or not.)
>> A state is an improper ancestor and descendent of itself. Proper ancestors and descendents are the ones that are distinct from the state. This use of 'proper' vs 'improper' is common in formal language theory, but I agree that there should be a definition in the spec.  

4. There exists a pseudo-code function "initializeDatamodel" that is used in two ways: initializeDatamodel(datamodel, doc) and initializeDataModel(datamodel.s,doc.s). I believe the former means "initialize the data model with respect to all states in the document" and the latter is "initialize the datamodel just for this state", but the parameters make little sense to me for the second. A comment or aside description would be welcome.

>> Agreed. I will add it. 

5. (very minor) Is there a compelling reason to differentiate "procedures" versus "functions" in the pseudo-code?
>> Functions return values, procedures do not.  People who like functional programming languages, including one of the people who worked on the algorithm, care about this distinction, so we are careful about it, even though a lot of people don't see the point of it.  

6. The Datatypes section in Appendix A describes the interface for List as having "function tail()      // Returns the tail of the list". The interpreter details makes it appear that the "tail" here is not the last element, but rather all except the head/first element. This comment should be clear about this.

>> Agreed.  This use of 'tail' will make sense to old LISP programmers, but won't be immediately clear to a lot of people.  

7. Appendix A mentions that there is a global variable "historyValue", and uses it as an associative hash throughout the code, but no code ever initializes this variable or sets its type. Presumably this should be set during the "interpret" procedure to a data type not listed in the Datatypes preamble.
>> This will need some thought.  

8. The function getChildStates() is used throughout the algorithm without definition. I cannot tell if this function is intended to return only "proper" states or also pseudo-state children. Please clarify both in email and the spec.
>> It should retun only <state> and <parallel> children.  I will add a clarification to the spec.  

9. The mainEventLoop includes the code "for inv in state.invoke: inv.invoke(inv)". Why is is this written as an object-oriented method invocation that takes the receiver as a parameter? I suggest this should either be "invoke(inv)" or "inv.invoke()".
>> We'll discuss this in the group.  

10. The specs do not explain the semantics of a "continue" statement during a "while" loop. Presumably this exits the loop at the current location and resumes at the top of the loop, as in "continue" in JavaScript, the "next" statement of Ruby, or a manual "goto" statement in Lua. It would be nice if the spec were clear about this.
>> It is intended to be the 'continue' of traditional languages, as you indicate.  I'll add a clarification.

11. Various "if" statements in the pseudo code have two colons at the end of the line, while others have only one colon. I'm assuming that the double-colons are typos and not indicative of some subtle behavioral change.
>> Yes, it's a typo.

12. The preemptsTransition function is confusing. It is used in exactly one spot in the code, passing the variables "(t2, t)". However, the signature of the function is written with swapped parameters, "(t1, t2)". But then, the body of the function uses the variables "t" and "t2". Not only is this broken, but it casts serious doubt on which transition is supposed to preempt the other. Please clarify this via email and fix the spec.
>> The body should use 't1' in place of 't'.  It should return true if t1 preempts t2 as defined in the prose above.  

13. I find the Type1/Type2/Type3 confusing for transitions. First, <transition> elements already have a "type" attribute that is unrelated; a better term (Category?) should be used. Secondly, the prose descriptions for both Type 2 and Type 3 do not seem rigorous. What is a "transition within a single child of <parallel>"? A transition that is in a state that is the sole child of a parallel? A single transition that is a direct child of a <parallel> node? Please revisit the preemptsTransition description and pseudo-code to make this subtle area for bugs robust and clear.
>> Yes, other people have commented that the definition of preemption is quite murky.  I'll have to work on clarifying it. "Category' is a better term than 'type' for the reason you mention. In the case of ""transition within a single child of <parallel>", it is a transition whose source and target are contained within a single <state> child of <parallel> and which can be taken without exiting that <state>.  
>> Another way of stating the issue is that two transition conflict if their exit sets (the set of states that they exit) have a non-null intersection.  In case of conflict, we execute the first transition (in document order) and preempt the second.  

14. Section C.2.1 shows invalid SCXML in the example; not only is the <scxml> element missing the xmlns, but the "CEO" element has incorrectly escaped the XML attribute values.
>> I'll look at this.  

15. Example G.3 includes (excerpted heavily) the markup: <state id="engine"><transition target="off"/><state id="off">…</state></state>. It would appear that every step of evaluation an engine that is in state "engine" should transition to "off"; as this is a substate of "engine", this is an endless loop. (In my likely-flawed implementation it certainly results in an infinite loop as the machine never gets to "stable=true".) Is this transition intended to be wrapped in an <initial> element instead? Or have I misunderstood and mis-implemented how the intepreter should handle such a situation?
>> I will need to look at this more carefully. I think you may be right that we need an <initial> element, or 'initial' attribute (since there's no executable content in the transition.)  The same holds for the transition to 'idle' inside the 'on' state.  

16. I could not discern what should occur for a machine whose content is a single <scxml> element with no state children. While so trivial as to be almost useless (except for unit tests that are testing the datamodel), this appears to be a legal state machine. However, given that this <scxml> element lacks an initial attribute or element (it s an atomic state, after all), the pseudo-code for interpret: "executeTransitionContent([doc.initial.transition] enterStates([doc.initial.transition])" is in error (there is no doc.initial). Guarding against this prevents the <scxml> state from ever being entered into the configuration, leaving the state machine in a null state.
>> Good question.  Very good question.  We'll have to think about this.  We could ban this by requiring that <scxml> have at least one <state> or <transition> child, but there may be reasons to allow it and define the behavior more carefully.  

17. In general, I strongly echo the request for a set of SCXML examples, example event input and the configurations and log output that should come from each. These would ideally range from the simple to (eventually) tests that exercise all aspects of the interpretation algorithm. But do not let the "perfect" be the enemy of the "good": any examples (and their expected behavior in the presence of a series of events) would be a boon.
>> We will provide a set of tests as part of the Candidate Recommendation version of  the spec.  

The current situation is a large set of interconnected rules described over (and cross-linked throughout) 20+ pages coupled with normative pseudo-code that can't actually be run and that has bugs that would prevent it from compiling/parsing if it were real code. Making the pseudo-code robust would be a nice step. Providing a normative reference implementation that can actually run (in ECMAScript, perhaps?) would be even better.
>> There is no official reference implementation.  However the implementer of PySCXML ( http://pyscxml.spyderbrain.com/ ) is a member of the SCXML working group and keeps his implementation as up-to-date as he can.  


To cap off all the complaining and mistakes, I'd like to thank everyone who has worked on this standard. By and large it is well-written and helpful. As one who writes standards and specifications for his job often, I appreciate that while mistakes and bugs will happen, a considerable amount of passionate effort appears to have been applied over the years. :)

--
Gavin Kistner
Product Designer
NVIDIA, Inc.

Received on Friday, 25 January 2013 15:02:46 UTC