- From: Zjnue Brzavi <zjnue.brzavi@googlemail.com>
- Date: Tue, 8 Apr 2014 10:57:00 +0100
- To: "www-voice@w3.org" <www-voice@w3.org>
- Message-ID: <CABmmmmx=eOJ1PEDbEoDrqCRADQEgJiAx83CmjGmHOg5UNETk2Q@mail.gmail.com>
Hi,
Very encouraging to see such improvements to the official interpretation
algorithm being shared for everyone's benefit.
Thanks to all involved. Looking forward to apply them if/when they are
applied to the specification.
Best regards,
Zjnue
On Wed, Apr 2, 2014 at 3:39 PM, Ate Douma <ate@douma.nu> wrote:
> On 02-04-14 16:10, Jim Barnett wrote:
>
>> If getTargetStates isn't needed to dereference history states, then it
>> can be
>> dropped altogether, because it is returning the same thing as
>> transition.target.
>>
>
> Indeed. Which is how I'm using it.
>
>
>
>> - Jim
>> On 4/2/2014 5:27 AM, Ate Douma wrote:
>>
>>> On 02-04-14 00:10, Jim Barnett wrote:
>>>
>>>> Ate,
>>>> There is a problem if getTargetStates does not dereference history
>>>> states.
>>>> computeEntrySet calls it to produce the initial list of states for
>>>> addDescendentStatesToEnter to expand. If getTargetStates doesn't
>>>> dereference
>>>> history states, they will get added to the statesToEnter list before
>>>> addDescendentStatesToEnter gets called - and history states should not
>>>> be on
>>>> statesToEnter. Furthermore, computeExitSet calls getTargetStates, and
>>>> it will
>>>> also need to know what the actual dereferenced target state set is.
>>>>
>>>
>>> Right. And I realize I already solved this differently, and IMO
>>> elegantly, in
>>> my implementation :)
>>>
>>> The 'hint' leading to my implementation was the fact that
>>> addDescendantStatesToEnter (also) stores the state parameter in
>>> statesToEnter.
>>> And is expected to do so as it recursively is invoked for derived states
>>> which
>>> were not yet initially added to the statesToEnter.
>>>
>>> What I thus did instead is the following (in pseudo language):
>>>
>>> procedure computeEntrySet(transitions, statesToEnter,
>>> statesForDefaultEntry)
>>> enterableTargets = new OrderedSet()
>>> historyTargets = new OrderedSet()
>>> for t in transitions:
>>> for s in getTargetState(t.target)):
>>> if isHistoryState(s):
>>> historyTargets.add(s)
>>> else
>>> enterableTargets.add(s)
>>> for s in enterableTargets:
>>> addDescendantStatesToEnter(s,statesToEnter,statesForDefaultEntry)
>>> for h in historyTargets:
>>> addDescendantStatesToEnter(h,statesToEnter,statesForDefaultEntry)
>>> // unmodified below
>>> for t in transitions:
>>> ancestor = getTransitionDomain(t)
>>> for s in getTargetStates(t.target)):
>>> addAncestorStatesToEnter(s, ancestor, statesToEnter,
>>> statesForDefaultEntry)
>>>
>>> This way, no history state is added to enterableStates because
>>> addDescendantStatesToEnter only does this for non-history states.
>>>
>>> Note that addAncestorStatesToEnter will work just fine with a history
>>> state as
>>> parameter.
>>>
>>> Finally, concerning computeExitSet: this also isn't a problem in my
>>> implementation because I already 'compile' the source of a history
>>> transition
>>> to that of its parent state, and actually (thus) can 'compile' the
>>> transitionDomain and the targets of a transition at document load time.
>>>
>>> In pseudo-language in the algorithm, this also can easily be catered for
>>> if
>>> you update getTransitionDomain as follows:
>>>
>>> function getTransitionDomain(t)
>>> tstates = getTargetStates(t.target)
>>> if not tstates
>>> return t.source
>>> state = t.source
>>> if isHistoryState(t.source):
>>> state = t.source.parent
>>> if t.type == "internal" and isCompoundState(state) and
>>> tstates.every(lambda s: isDescendant(s,state)):
>>> return state
>>> else:
>>> return findLCCA([state].append(tstates))
>>>
>>> The above can further be optimized by filtering out the history targets
>>> from
>>> being passed to the lambda expression or from the findLCCA(stateList),
>>> but it
>>> isn't necessary either.
>>>
>>> With the above minor improvements, the getTargets(transition) as well as
>>> the
>>> getTransitionDomain(transition) procedures can remain 'stateless' (not
>>> history
>>> state aware) and can be compiled at load time.
>>> If you add history dereferencing to getTargets(transition) this no
>>> longer will
>>> be possible, so IMO is something we should try not to do.
>>>
>>>
>>>> It's a big gap in the algorithm that getTargetStates isn't defined - I'm
>>>> assuming that it was in an earlier version, and got lost somewhere.
>>>>
>>>> In any case we need multiple fixes so that: 1) history states don't end
>>>> up on
>>>> the statesToEnter list and 2) default history executable content gets
>>>> recorded.
>>>>
>>>
>>> See my above proposed changes, AFAICT it solves 1) elegantly and then 2)
>>> no
>>> longer is an issue (already solved by the earlier update to the
>>> algorithm).
>>>
>>> Regards,
>>> Ate
>>>
>>>
>>>> - Jim
>>>> On 4/1/2014 2:45 PM, Ate Douma wrote:
>>>>
>>>>> On 29-03-14 20:26, Zjnue Brzavi wrote:
>>>>>
>>>>>> On Sat, Mar 29, 2014 at 1:24 PM, Jim Barnett <1jhbarnett@gmail.com
>>>>>> <mailto:1jhbarnett@gmail.com>> wrote:
>>>>>>
>>>>>> Zjnue,
>>>>>> I agree that we need more tests, but why would
>>>>>> defaultHistoryContent
>>>>>> need
>>>>>> to get passed to getTargetStates? It's just an accessor function.
>>>>>> Though I
>>>>>> do think we have to consider how getTransitionDomain and findLCCA
>>>>>> should
>>>>>> handle history states. They ignore them at the moment, and
>>>>>> _maybe_ that's
>>>>>> ok...
>>>>>>
>>>>>> - Jim
>>>>>>
>>>>>>
>>>>>> Hi Jim,
>>>>>>
>>>>>> Perhaps as quick background, my implementation follows (atm) the
>>>>>> proposed
>>>>>> algorithm pretty much verbatim, in order to aid debugging for cases
>>>>>> like this
>>>>>> and to make getting started with SCXML a little easier for myself and
>>>>>> others.
>>>>>>
>>>>>> Simply put, the recently published changes to the algorithm for
>>>>>> defaultHistoryContent support are not satisfying test579.
>>>>>>
>>>>>> While the getTargetStates function seems pretty harmless, it is also
>>>>>> hisory-aware and can redirect a transition target.
>>>>>>
>>>>>
>>>>> Hi Zjnue,
>>>>>
>>>>> Sorry for chiming in this late, and going back to this earlier
>>>>> message, but
>>>>> I'm puzzled by the above statement.
>>>>>
>>>>> I can find no indication in the algorithm nor in the specification
>>>>> text that
>>>>> the getTargetStates function is supposed to do anything more than just
>>>>> returning the direct target(s) of a transition.
>>>>>
>>>>> It seems to me that the problems you encountered are actually *caused*
>>>>> by the
>>>>> the history-awareness and dereferencing in your getTargetStates
>>>>> implementation.
>>>>>
>>>>> AFAICT the pseudo algorithm expects it will take care of the history
>>>>> dereferencing itself in the addDescendantStatesToEnter procedure.
>>>>> If you do this upfront in you getTargetStates implementation, the
>>>>> addDescendantStatesToEnter never will get to 'see' a history state as
>>>>> it is
>>>>> supposed to, and neither can it then handle the default history
>>>>> content as
>>>>> just was added.
>>>>>
>>>>> I just completed the implementation of the current algorithm for Apache
>>>>> Commons SCXML (although not yet committed), and I now can successfully
>>>>> run
>>>>> test579, so it seems to me the proposed change simply works as
>>>>> expected.
>>>>>
>>>>> But maybe I'm overlooking some other specification requirements for
>>>>> getTargetStates?
>>>>> I'd appreciate some pointers in that case :)
>>>>>
>>>>> Regards, Ate
>>>>>
>>>>>
>>>>> In the case of test579, the <initial> transition, targets <history
>>>>>> id="sh1">,
>>>>>> but is effectively redirected to <state id="s01" >, skipping any
>>>>>> executable
>>>>>> content of <history> along the way.
>>>>>>
>>>>>> Please take a look at my code for getTargetStates and getTargetState
>>>>>> below
>>>>>> [1].
>>>>>>
>>>>>> It is only with the following two lines present in getTargetState:
>>>>>> if( defaultHistoryContent != null &&
>>>>>> h.parent.exists("id") )
>>>>>> defaultHistoryContent.set(h.parent.get("id"),
>>>>>> h.transition().next());
>>>>>> ..that 579 passes.
>>>>>>
>>>>>> As mentioned, I did not look at it in depth and will not be able to
>>>>>> spend more
>>>>>> time on it over the next few days unfortunately.
>>>>>>
>>>>>> So in summary, after adding the published additions to the algorithm,
>>>>>> test579
>>>>>> did not pass.
>>>>>> It was only after the changes to getTargetStates that it did.
>>>>>> Hope this helps for now.
>>>>>>
>>>>>> Thanks,
>>>>>> Zjnue
>>>>>>
>>>>>> [1]
>>>>>>
>>>>>> function getTargetStates( node : Node, ?defaultHistoryContent :
>>>>>> Hash<Iterable<Node>> ) : List<Node> {
>>>>>> var l = new List<Node>();
>>>>>> if( !node.exists("target") )
>>>>>> return l;
>>>>>> var ids = node.get("target").split(" ");
>>>>>> var top = node;
>>>>>> while( top.parent != null && !top.isTScxml() )
>>>>>> top = top.parent;
>>>>>> for( id in ids ) {
>>>>>> var ts = getTargetState(top, id, defaultHistoryContent);
>>>>>> for( tss in ts )
>>>>>> l.add( tss );
>>>>>> }
>>>>>> return l;
>>>>>> }
>>>>>>
>>>>>> function getTargetState( s : Node, id : String,
>>>>>> ?defaultHistoryContent :
>>>>>> Hash<Iterable<Node>> ) : List<Node> {
>>>>>> if( s.get("id") == id )
>>>>>> return [s].toList();
>>>>>> else {
>>>>>> for( child in s.getChildStates() ) {
>>>>>> var ss = getTargetState(child, id,
>>>>>> defaultHistoryContent);
>>>>>> if( ss != null )
>>>>>> return ss;
>>>>>> }
>>>>>> for( h in s.history() ) {
>>>>>> var hh = getTargetState(h, id);
>>>>>> if( hh != null ) {
>>>>>> if( defaultHistoryContent != null &&
>>>>>> h.parent.exists("id") )
>>>>>> defaultHistoryContent.set(h.parent.get("id"),
>>>>>> h.transition().next());
>>>>>> return historyValue.exists( h.get("id") ) ?
>>>>>> historyValue.get( h.get("id") ) :
>>>>>> getTargetStates( h.transition().next(),
>>>>>> defaultHistoryContent );
>>>>>> }
>>>>>> }
>>>>>> }
>>>>>> return null;
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 3/28/2014 8:51 PM, Zjnue Brzavi wrote:
>>>>>>
>>>>>>>
>>>>>>> I've added test579 to the suite. Let me know if there are
>>>>>>> problems
>>>>>>> with it or if you think there need to be more/different
>>>>>>> tests.--
>>>>>>>
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Not had a very good look yet, but it appears we'll need to pass
>>>>>>> defaultHistoryContent to getTargetStates as well, as it is
>>>>>>> another area
>>>>>>> where the algorithm is skipping past some history elements
>>>>>>> without
>>>>>>> considering their executable content. This diff passes test579 :
>>>>>>> https://github.com/zjnue/hscxml/commit/
>>>>>>> 4b9091470d3c7c1995e6b41900535e0437ec3eb4
>>>>>>>
>>>>>>> Another test or 2 on the subject is probably a good idea.
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Zjnue
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> Jim Barnett
>>>>>> Genesys
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>
Received on Tuesday, 8 April 2014 09:57:29 UTC