Support for Copying Behaviors?

[amalnanavati]

I'd like to be able to use the same behavior in multiple parts of a tree (e.g., calling a ROS service that toggles some robot functionality on/off multiple times in the same tree). #281 disallowed this, because it may be problematic if behaviors maintain state.

I don't mind re-creating another version of the same behavior, but I want to do so in a way that maximizes code re-use. I see two options:

1. Put the logic for creating the behavior into a helper function, and call it every time I want that behavior in the tree.
2. Create the behavior once, and copy/clone it when I want to re-use it.

In terms of the second option, I was wondering:

1. Would copy.deepcopy work on a behavior? I imagine not, because things like the parent attribute would also be copied over, which is what gives rise to the error in [composites] protect against multiple parents #281 .
2. If not, what do you think of adding support for copying/cloning a behavior? Such a function would ideally copy the functional attributes of the behavior (e.g., any attributes defined by the subclass, any children it has) but not copy the attributes that are assigned when it is put into another tree.

[stonier]

There's lots of nuances to this once you get beyond simple objects.

On the subject of Copies

A copy or deepcopy should logically give you a copy of the object in it's current state, so:

class Foo(behaviour.Behaviour):
    def __init__(self, initial_value):
        self.counter = initial_value
        
    def update():
       self.counter += 1

foo = Foo(5)
foo.copy()  # a copy here should give you an object with self.counter = 5
foo.tick()
foo.tick()
foo.copy()   # a copy here should give you an object with self.counter = 7

Q: Do you want constructed copies or runtime copies? Does it matter (i.e. if you want constructed copies, calling it before you tick it is sufficient)?

What about init() logic?

Suppose you want copies, but you have a behaviour like this:

class Foo(behaviour.Behahviour):
    def __init__(self, channel_name):
        connect_to_some_middleware_channel(channel_name, self.callback)

   def callback(self):
       println("May he reach out and bless you with a tickle from his noodly appendages")
       
foo = Foo("noodles")
bar = foo.copy()           # a non-functional copy - no channel has been setup

The copy skips the business logic happening in __init__ - probably not what you want, potentially very surprising and great fuel for bugs. Creating threads and the like suffer from similar problems.

You might do things a little differently:

class Foo(behaviour.Behahviour):
    def __init__(self, channel_name):
        self.channel_name = channel_name

    def setup(self):
        connect_to_some_middleware_channel(self.channel_name, self.callback)

   def callback(self):
       println("May he reach out and bless you with a tickle from his noodly appendages")

foo = Foo("noodles")
bar = foo.copy()          # a non-functional copy - no channel has been setup
bar.setup()                   # aha, now you are cooking, but this is a 'magic' step

bar.tick()
foobar = bar.copy()
foobar.setup()             # ok here, but what if bar.tick() does things that break a future call on setup()?

You're depending on a user to keep their init() very clean and a call to setup() here. That might be ok for a specific behaviour in actual user code, but copy() is not something I'd be comfortable about generalising and building into the py_trees framework as it's leaving users with potentially non-functional copies that require them to follow certain 'magic' steps to get it functional. Even after that it may still be non-functional (e.g. if the channel construction was in init().

What about Blackboards?

This is another problem - the blackboard can often be an indirect form of state for a behaviour. Do you copy that too? It can be a right thing to copy it in some situations and an entirely wrong thing to do so in other situations.

On the subject of Clones

The distinction between copy and clone is usually about whether or not you need to program some logic above and beyond a brute force copy. That could help you implement logic usually hidden in init(), but it requires a behaviour to do some extra programming.

---

So what to do.

The concept of copying is terribly problematic. I'd avoid this at all cost.

I could imagine implementing a clone() method in behaviour.Behaviour that by default, throws an exception. Child classes could override it and put their cloning logic inside. Most of the behaviours in py_trees and py_trees_ros could probably implement this method. It's worth noting, that you might probably never be able to rely on every behaviour implementing this method though, but nonetheless, it could still be useful.

A helper function to help you create objects is a perfectly fine thing. I very often use the idiom pattern to create identical subtrees that need to be inserted in multiple locations across a complex tree.

Another point that might be pertinent - try to avoid instantiating duplicate ROS objects across the tree. This gets very heavy. One way to avoid this is to split your behaviours - put ROS machinery in one and programming logic in the other. Use the blackboard to connect them. e.g.

• Put a reset blackboard variable behaviour early in the tree. Make sure this behaviour always fired.
• In the tree, use pythonic behaviours at various locations. If they're fired, they write to that variable.
• At the end of the tree, have a ROS behaviour that checks the blackboard variable. If it's set, call a ROS service/action/publisher. Make sure this this behaviour is always fired.

You can flip that around too and have ROS behaviours up front that write data to blackboard variables. Then pythonic behaviours in the tree make use of that data. This can save you having to install N listeners for behaviours that are cloned across the tree and may or may not be fired. See tutorial two. You could have that battery emergency subtree in multiple locations across the tree, but because Battery2BB is up front, there's only one listener.

[amalnanavati]

Thank you for the detailed response. I agree with what you highlighted about the challenges of implementing a copy function since it would require users to follow ‘magic’ steps to enable their behavior to work with it.

I like the idea of a clone function that subclasses of Behavior can/should implement, and think it would be a good approach for my situation. However, I will detail a representative example of my situation below in case you have another suggestion for addressing it.

---

In terms of standalone ROS nodes, I have MoveIt2’s MoveGroup running. I want a tree that performs two consecutive motions -- the first ignoring a particular collision object in MoveIt's planning scene (done via service calls to MoveIt), and the second respecting that collision object. Hence, my “main tree” is:

SequenceWithMemory[
    ToggleCollisionObjectOff, 
    MoveToA,
    ToggleCollisionObjectOn, 
    MoveToB,
]

However, say there is a failure in MoveToA. I still want to perform cleanup by toggling the collision object back on, before passing the failure up the tree (i.e., like a Python finally statement). So my final tree looks like:

SelectorWithMemory[
    SequenceWithMemory[
        ToggleCollisionObjectOff, 
        MoveToA,
        ToggleCollisionObjectOn, 
        MoveToB,
    ]
    SuccessIsFailure[ToggleCollisionObjectOn]
]

Hence, I need to use the ToggleCollisionObjectOn behavior multiple times in the tree.

I currently use an idiom to create ToggleCollisionObject behaviors, but it takes many lines of code to create the behavior twice. The idiom needs to take in the behavior's name, the ROS node to associate the service client with, the collision_object_id, whether to allow or disallow collisions, and the logger for that behavior. Calling this idiom multiple times in the tree is 5+ lines of code each time (using Black formatter and pylint). If, on the other hand, I could call a clone function (perhaps taking in a new name as an argument), then I can set the parameters of the behavior just once (with 5+ lines of code) and then call a single line to duplicate it.

The core issue here is that I’ve been making idioms generic enough to be used across multiple trees; hence they take in lots of parameters. However, when I want to reuse a behavior within a single tree, (nearly) all the parameters stay the same, so it seems overkill to call the whole idiom again.

[amalnanavati]

As I was writing the above comment, I realized that Python partial functions can address what I need. So there is at least one way around this without a behavior clone functionality.

With that said, I’d still love any suggestions you have on the tree structure, particularly on:

1. How to achieve this finally style of cleanup behavior;
2. How to avoid instantiating replicate ROS objects in this case. Each ToggleCollisionObject behavior has its own ROS service client, and each MoveTo call has its own ROS action client. The only way I can see around that is putting those service/action clients on the Blackboard, but that seems against the spirit of the Blackboard (and deviates from how py_trees_ros does it).

Thanks again!

[stonier]

1. finally

You can get this behaviour with something like:

|| Parallel
  - Moving Sequence
    + ...
  + Finally

with some implementation in Finally's terminate() method:

class Finally(behaviour.Behaviour)
   def update():
      return RUNNING

    def terminate():
        // code that toggles the collision object

When the sequence completes the parallel will invalidate Finally, triggering it's terminate() method and the parallel will reflect the sequence's result.

That doesn't let you reuse behaviours though. One idea that we could implement in py_trees is to use a decorator.

|| Parallel
  - Moving Sequence
    + ...
  ^ Finally Decorator
    + Toggle Collision Object On

The decorator would always return running, and if invalidated, it would call the child's update() method from it's own terminate() method. Then invalidate the child.

One nice advantage here - it's very easy to read from the tree what's happening.

[stonier]

The action clients aren't easy to move around like I suggested earlier - I should have mentioned that. The principle of shifting ROS mechanisms to the front and back of the tree mainly applies to publishers, subscribers and sometimes services.

In this case, the toggle collision object service doesn't actually need to be called right there and then in the middle of your tree tick. You could reset/write a boolean to the blackboard with (Un)SetBlackboardVariable in the tree and at the end have a behaviour that reads from the blackboard and calls the service.

[amalnanavati]

Got it, this all makes sense. Handling the finally logic in a terminate function makes sense.

Thanks for starting the PR! I have some thoughts, which I'll put in the PR directly.