splintered-reality / py_trees

Python implementation of behaviour trees.
Other
440 stars 150 forks source link

Parallel Policy doesnt work as expected. #433

Open mcelhennyi opened 11 months ago

mcelhennyi commented 11 months ago

I am trying to make use of the SuccessOnSelected Policy for the parallel node. I have three nodes. Two nodes I do not care about them returning success, one node I care about its return (i want success). I have created a test project to emulate my system, and when I run it I do not ever get the success from the selected node to create a success on the parallel node.

import py_trees
from py_trees.behaviour import Behaviour
from py_trees.trees import BehaviourTree
from py_trees.common import Status, ParallelPolicy
import py_trees.tests
import time

class AlwaysFail(Behaviour):

    def update(self) -> Status:
        return Status.FAILURE

class SucceedAfter(Behaviour):

    def __init__(self, name: str, succeed_time = 5):
        super().__init__(name)
        self._after_time = succeed_time
        self._start_time = time.time()

    def update(self) -> Status:

        if time.time() - self._start_time >= self._after_time:
            print("SUCCESS")
            return Status.SUCCESS

        print("FAILURE")
        return Status.FAILURE

always_fail_1 = AlwaysFail("fail-1")
always_fail_2 = AlwaysFail("fail-2")
succeed_after = SucceedAfter("succeed_after")

root = py_trees.composites.Parallel(
    name="Parallel",
    policy=ParallelPolicy.SuccessOnSelected(children=[succeed_after]),
)
root.add_child(always_fail_1)
root.add_child(always_fail_2)
root.add_child(succeed_after)

tree_bt = BehaviourTree(
    root
)

while True:

    tree_bt.tick()

    if tree_bt.root.status == Status.SUCCESS:
        print("TREE SUCCESS")
        break
    else:
        print("TREE Failure")

    time.sleep(1)

I expect to see the Node print "Success" and the program to exit just after due to the root node status also being "Success".

Upon reading the code for the Parallel composite policy section, it seems like the way its written, that the only way the policies are checked are when the children nodes are all status != Failure. The line here utilizes the python next() function, which will only throw the StopIteration exception when the list its iterating over is empty/fully iterated through. The list this implementation is iterating over is a list of all nodes that are status == Failure. The only way this will throw the exception is if NO nodes are in a Failure state.

If my policy is to be successful when only a single specific node is Success, then this will never fire.

Am I misusing this library, or is this in fact a bug?

mcelhennyi commented 11 months ago

Upon further reading/inspection I now realize I am misinterpreting what the intention of a parallel node is and what SuccessOnSelected really means. The docs for SuccessOnOne reads that the "one" being a success implies that the others are not finished yet (aka still RUNNING status). This "rule" applies to all policies (although not stated in the description for SuccessOnSelected)

The issue for me was that the first two branches of my tree would return success/failure depending on the situation, but never running. However the way the policy evaluation is written here, if any of the nodes return failure that means that the policies are never checked.

I think it would make more sense to allow that to be configurable: Either the parallel node checks the policies EVERY tick or ONLY when the nodes are running/successful.

Leaving open now as a feature request.