Use behaviour trees in scriptlets demo #208

BMarchi · 2019-07-25T15:53:03Z

This demo shows how to use a pre/pos tick handler with objects and plain functions, while also including a object that can do anything when a decorator calls it

hidmic · 2019-07-26T18:47:18Z

delphyne-demos/demos/scriptlets.py

@@ -75,8 +74,16 @@ def record_tick(self):
            self.reset()
        self.start()

+    def pos_tick_handler(self, behaviour_tree):
+        if self._current_start_time is None:
+            self.start()


@BMarchi SimulationStats.start() should be called before the first tick happens.

hidmic · 2019-07-26T18:48:45Z

delphyne-demos/demos/scriptlets.py

+        if self._current_start_time is None:
+            self.start()
+        else:
+            self.record_tick()


@BMarchi if SimulationStats is completely initialized by the time a post tick handler gets called, we don't need the additional method and we can simple use record_tick() or a lambda to drop any unused arguments.

hidmic · 2019-07-26T18:54:25Z

delphyne-demos/demos/scriptlets.py

+            speed=4.0
+        )
+    decorator = delphyne.decorators.additional_action.AdditionalAction(
+        child=rail_car, conditional_object=monitor)


@BMarchi this decorator seems a bit non-idiomatic to me. I would've imagined something like an e.g. WaitUntil behaviour and an e.g. SetSpeed behaviour, both children of a Sequence composite decorated by a OneShot decorator.

The first issue will be that the SetSpeed behaviour would require to reference the agent by using a call to parent->parent to pass the decorator or using a blackboard, which seems an overkill.
When I created this decorator, I had on my mind to do any kind of stuff with the decorator's child. We could just change the name's value of the decorator if we require to do other stuff, instead of creating a ton of different behaviours just to do set_speed = 20 for instance

The first issue will be that the SetSpeed behaviour would require to reference the agent by using a call to parent->parent to pass the decorator or using a blackboard, which seems an overkill.

Hmm, can't we give it a reference to the agent behaviour during subtree construction? Instead of traversing the tree I mean. I agree though that this has somewhat less to do with behaviour and more with structure, so it looks funny.

When I created this decorator, I had on my mind to do any kind of stuff with the decorator's child. We could just change the name's value of the decorator if we require to do other stuff, instead of creating a ton of different behaviours just to do set_speed = 20 for instance

Hmm not convinced the potential for reuse is enough to bypass the tree (which IMO is what we're doing here in TimeMonitor.do_action(), replacing an entire subtree with code). That's why I said it's non-idiomatic, as opposed to non functional. How would you augment this to follow a speed profile over time, braking in several places and steering the vehicle?

Hmm, can't we give it a reference to the agent behaviour during subtree construction?

We can. I fear it could lead to a bunch of behaviours touching the same thing.

braking in several places and steering the vehicle?

To me, that's the point of blackboards. Keep track of the variables modified by behaviours and use them when you need it in any moment (i.e behaviours). Since we have a global one it can get big in no time, but I'm thinking that having an object that it's related to one and only one agent in the blackboard where we can save the variables we want, is the way to go. The big picture for me is that any child of the agent's behaviour should monitorize something and save somewhere the result, so in the next tick the agent is "aware" of that thanks to the blackboard.

We can. I fear it could lead to a bunch of behaviours touching the same thing.

Hmm, I'm not sure I follow why that is a problem. It is problematic in the sense that not all relationships are explicit in the tree, but that's not the case right now either nor is the case if several behaviours, be them children or siblings, share state in the blackboard,

To me, that's the point of blackboards. Keep track of the variables modified by behaviours and use them when you need it in any moment (i.e behaviours).

Sure, that makes sense for state, but what about behaviour? If we express behaviour in the tree, it's explicit and easy to compose, if we do it through code to boost reuse we are back in procedural programming land.

Hmm, I'm not sure I follow why that is a problem.

Tons of behaviours modifying the agent directly will make things more complex for someone who didn't write the code rather than modifying the agent in just one place. I don't know how this can impact in understanding the tree if we have a tree with a considerable depth where a child modifies a parent that it's way above it instead of touching a variable and let anyone read it if they need to.

it's explicit and easy to compose

I agree with adding behaviours, using composites and such. What I don't think it's good is what I said. Modify a parent by a child that can be 5, 10 or even more levels of depth instead of touching a variable in the blackboard and let the others use it as they need.

hidmic · 2019-08-02T14:09:29Z

delphyne-demos/demos/scriptlets.py

-def random_print():
+'''
+behaviour_tree parameter is necessary to add it as a pos/pre tick handler
+'''


@BMarchi nit: put this in the function's docstring.

hidmic · 2019-08-02T14:10:53Z

delphyne-demos/demos/scriptlets.py

+            print("Running simulation indefinitely.")
+            stats.start()
+            simulation_tree.tick_tock(
+                period=time_step)


@BMarchi nit: why the line wrap?

hidmic · 2019-08-02T14:11:06Z

delphyne-demos/demos/scriptlets.py

        else:
            # run for a finite time
            print("Running simulation for {0} seconds.".format(
                args.duration))
-            runner.run_async_for(args.duration, launcher.terminate)
+            stats.start()
+            simulation_tree.tick_tock(


@BMarchi nit: same about line wraps

hidmic · 2019-08-02T14:11:51Z

delphyne-demos/demos/scriptlets.py

@@ -35,6 +37,29 @@
 ##############################################################################


+class ChangeSpeed(py_trees.behaviour.Behaviour):
+
+    def __init__(self, parent_name, speed=1.0,


@BMarchi nit: parent_name -> agent_name

hidmic · 2019-08-02T14:13:51Z

delphyne-demos/demos/scriptlets.py

+            speed=4.0
+        )
+
+    change_speed = ChangeSpeed('rail0', 20.0)


@BMarchi nit: use keyword arguments for improved readability

hidmic · 2019-08-02T14:20:42Z

delphyne-demos/demos/scriptlets.py

+
+    timeout_decorator = py_trees.decorators.Timeout(
+        child=change_speed,
+        duration=5.0)


@BMarchi It seems the standard Timeout decorator uses system time. We cannot rely on the real time factor to be 1, thus we can't use it. Also, using a timeout to instrument a wait is a bit odd. Could the new decorator/behaviour have wait/delay/until in the name?

hidmic · 2019-08-02T14:21:53Z

delphyne-demos/demos/scriptlets.py

+            parent_attributes = blackboard.get(self.parent_name)
+            parent_attributes["speed"] = self.speed
+            blackboard.set(self.parent_name, parent_attributes, True)
+            self.status = py_trees.common.Status.SUCCESS


@BMarchi if you move away from Timeout to a Wait maybe this can migrate to update().

hidmic · 2019-08-02T14:23:05Z

delphyne-demos/demos/scriptlets.py

+            blackboard = py_trees.blackboard.Blackboard()
+            parent_attributes = blackboard.get(self.parent_name)
+            parent_attributes["speed"] = self.speed
+            blackboard.set(self.parent_name, parent_attributes, True)


@BMarchi this is a bit verbose, can we wrap it somehow? Assuming you chose not to simply use the agent for X reasons.

hidmic

LGTM for a few nits.

hidmic · 2019-08-07T15:13:34Z

delphyne-demos/demos/scriptlets.py

+            simulation.get_current_time() >= 10.0:
+            print("Speed up!")
+            blackboard = py_trees.blackboard.Blackboard()
+            parent_attributes = blackboard.get(self.agent_name)


@BMarchi nit: parent_attributes -> agent_attributes

hidmic · 2019-08-07T15:14:49Z

delphyne-demos/demos/scriptlets.py

+
+    """
+    Change speed of agent_name after 10 seconds passed in the simulation.
+    """


@BMarchi this is good as-is, but just out of curiosity, why did you decide not to have a generic Delay behaviour tied to simulation time?

By behaviour you mean a decorator, right?

I guess you could do it either way. A decorator probably makes more sense, yes.

hidmic · 2019-08-07T15:15:55Z

delphyne-demos/demos/scriptlets.py

+
+    one_shot_decorator = py_trees.decorators.OneShot(
+        child=change_speed,
+        policy=py_trees.common.OneShotPolicy.ON_COMPLETION)


@BMarchi if we're using the OneShot decorator here, why do we need the speed_changed boolean flag on DelayChangeSpeed?

I saw that the update method was getting fired anyway, I'll double check again

hidmic · 2019-08-07T15:17:35Z

delphyne-demos/demos/scriptlets.py

+
+    change_speed = DelayChangeSpeed(agent_name='rail0', speed=20.0)
+
+    one_shot_decorator = py_trees.decorators.OneShot(


@BMarchi nit: consider collapsing behaviour initialization so that it reads:

change_speed = py_trees.decorators.OneShot(child=DelayChangeSpeed(...), ...)

hidmic · 2019-08-07T15:18:43Z

delphyne-demos/demos/scriptlets.py

@@ -75,9 +107,15 @@ def record_tick(self):
            self.reset()
        self.start()

+    def pos_tick_handler(self, behaviour_tree):
+        self.record_tick()


@BMarchi nit: if you use lambda _: stats.record_tick() for a post tick handler, there's no need for this tailor-made method.

I left it as it is just to show an example

Oh, I see. Nevermind then.

hidmic · 2019-08-07T15:19:19Z

delphyne-demos/demos/scriptlets.py

 from . import helpers

 ##############################################################################
 # Supporting Classes & Methods
 ##############################################################################


+class DelayChangeSpeed(py_trees.behaviour.Behaviour):


@BMarchi nit^2: consider DelayedChangeSpeed instead.

hidmic

LGTM pending green CI

hidmic · 2019-08-12T13:11:38Z

delphyne-demos/demos/scriptlets.py

-            runner.run_async_for(args.duration, launcher.terminate)
+            stats.start()
+            simulation_tree.tick_tock(period=time_step,
+                number_of_iterations=args.duration/time_step)


@BMarchi mind to add launcher.terminate() considering #213 (comment) ?

Hmm, no, let's merge and do so in a follow-up PR.

hidmic

LGTM

* Rename scriptlets maliput package for roads (#206) * Use behaviour trees in scriplets demo * Use behaviour trees delphyne-city demo (#209)

* Fix bad python import in delphyne_gui. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Refactor delphyne-dragway to use new behaviour tree-based API. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Add behaviour trees to gazoo example * Tidy up, functionalize subtree creation * Add termination after time is up * Refactor delphyne-crash to use new behaviour tree-based API. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Update some delphyne-* demos to use delphyne.behaviours.roads Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Use behaviour trees in scriptlets demo (#208) * Rename scriptlets maliput package for roads (#206) * Use behaviour trees in scriplets demo * Use behaviour trees delphyne-city demo (#209) * Polished demos refactored to use behaviour trees. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Use same time step for all demos. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

hidmic reviewed Jul 26, 2019

View reviewed changes

hidmic suggested changes Aug 2, 2019

View reviewed changes

hidmic approved these changes Aug 7, 2019

View reviewed changes

hidmic approved these changes Aug 8, 2019

View reviewed changes

JShep1 mentioned this pull request Aug 8, 2019

Add behaviour trees to gazoo example #213

Merged

hidmic reviewed Aug 12, 2019

View reviewed changes

hidmic approved these changes Aug 13, 2019

View reviewed changes

BMarchi force-pushed the bmarchi/scriptlets_pytree branch from 3d27b95 to 2ea708e Compare August 13, 2019 18:05

BMarchi added 4 commits August 13, 2019 15:10

Rename scriptlets maliput package for roads (#206)

d4b3233

Use behaviour trees in scriplets demo

06dd190

Use behaviour trees delphyne-city demo (#209)

ae0cad2

Rename imported python package

f175abb

BMarchi force-pushed the bmarchi/scriptlets_pytree branch from 2ea708e to f175abb Compare August 13, 2019 18:28

BMarchi merged commit fe11b1f into hidmic/py_trees-based-api Aug 13, 2019

BMarchi deleted the bmarchi/scriptlets_pytree branch August 13, 2019 18:37

hidmic pushed a commit that referenced this pull request Aug 20, 2019

Use behaviour trees in scriptlets demo (#208)

99f6ae6

* Rename scriptlets maliput package for roads (#206) * Use behaviour trees in scriplets demo * Use behaviour trees delphyne-city demo (#209)

hidmic pushed a commit that referenced this pull request Aug 23, 2019

Use behaviour trees in scriptlets demo (#208)

ec1d321

* Rename scriptlets maliput package for roads (#206) * Use behaviour trees in scriplets demo * Use behaviour trees delphyne-city demo (#209)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use behaviour trees in scriptlets demo #208

Use behaviour trees in scriptlets demo #208

BMarchi commented Jul 25, 2019

hidmic Jul 26, 2019

hidmic Jul 26, 2019

hidmic Jul 26, 2019

BMarchi Jul 29, 2019

hidmic Jul 29, 2019

BMarchi Jul 29, 2019 •

edited

Loading

hidmic Jul 29, 2019

BMarchi Jul 29, 2019 •

edited

Loading

hidmic Aug 2, 2019

hidmic Aug 2, 2019

hidmic Aug 2, 2019

hidmic Aug 2, 2019

hidmic Aug 2, 2019

hidmic Aug 2, 2019 •

edited

Loading

hidmic Aug 2, 2019

hidmic Aug 2, 2019

hidmic left a comment

hidmic Aug 7, 2019

hidmic Aug 7, 2019

BMarchi Aug 7, 2019

hidmic Aug 8, 2019

hidmic Aug 7, 2019

BMarchi Aug 7, 2019

hidmic Aug 7, 2019

hidmic Aug 7, 2019

BMarchi Aug 7, 2019

hidmic Aug 8, 2019

hidmic Aug 7, 2019

hidmic left a comment

hidmic Aug 12, 2019

hidmic Aug 12, 2019

hidmic left a comment


		change_speed = DelayChangeSpeed(agent_name='rail0', speed=20.0)

		one_shot_decorator = py_trees.decorators.OneShot(

Use behaviour trees in scriptlets demo #208

Use behaviour trees in scriptlets demo #208

Conversation

BMarchi commented Jul 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BMarchi Jul 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BMarchi Jul 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hidmic Aug 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

BMarchi Jul 29, 2019 •

edited

Loading

BMarchi Jul 29, 2019 •

edited

Loading

hidmic Aug 2, 2019 •

edited

Loading