Idiomatic way to loop tree logic

[samouwow]

Hi there,

I've been using forester for the last few days, nice work creating a library that's well documented with a full feature set.

My use case will involve re-running the same tree several times a second, and so it would be excellent for the library to support this in an efficient way. Crucially, I would like to avoid restarting the daemons each time the tree is re-run.

I've outlined a few potential designs for how this might be achieved below, roughly in order of usability I think they'd expose to the user:

1. Expose API for forester_rs::runtime::forester::Forester to separate ticking the tree from cleaning up the tree, which could be done either by:
   1. Exposing a way to manually tick the tree (and so use forester less as a framework and more as a standalone behavior tree library), e.g.

   // Build the tree
   let mut forester = fb.build().unwrap();
   
   // Tick until Success or failure is reached
   let mut tick_result = forester_rs::runtime::TickResult::Running;
   while tick_result == forester_rs::runtime::TickResult::Running {
       tick_result = forester.tick().unwrap();
       // Do other jobs here...
   }
   
   // Shutdown the HTTP server and daemons
   forester.cleanup().unwrap();

   2. Store a flag in the Forester struct to avoid shutting down the HTTP server and daemons after run() returns (I think this would be the easiest to implement), e.g.

   // Disable automatic cleanup
   fb.use_manual_cleanup();
   
   // Build the tree
   let mut forester = fb.build().unwrap();
   
   // Run until failure is reached
   while forester.run().unwrap() == forester_rs::runtime::TickResult::Success {
       // Rerun the tree at 20Hz
       std::thread::sleep(std::time::Duration::from_millis(50));
   }
   
   // Shutdown the HTTP server and daemons
   forester.cleanup().unwrap();

2. Implement a KeepRunningUntilFailure decorator, as implemented by BehaviorTree.CPP.
   1. This is my personal least favorite solutions, since the daemons will be terminated without any context of why the tree exited.
   2. This could work if it was combined with manual cleanup of daemons (like above) or perhaps if the result of forester.run() was made available to daemons.

I'm more than happy to contribute this change as a PR myself, I just wanted to collaborate on which method you thought would be most idiomatic for Forester. I'm personally in favour of 1.ii followed by 1.i.

I've also got a handful of minor improvements I've noted while using the library, if you're receptive to it I'm happy to submit these as PRs.

[besok]

Hi there!
That is great to hear. Thanks.
Sure you can provide any changes as you would find useful.

Regarding the given options I will be able to take a look in the evening.

[besok]

Yeah, It is hard to estimate the best solution here as I don't know the goal of the task. Can you elaborate on it maybe?
The thing is initially, I thought a pattern like that should be implemented using BT itself:

sequence repeat_until(fn:tree){
  ...
  fn(..)
}

// where x can depend on some logic inside the tree 
root main repeat_until(
   main_tree(x=1)
)

Meanwhile, my thoughts:

Expose API for forester_rs::runtime::forester::Forester to separate ticking the tree from cleaning up the tree, which could be done either by:

The idea, in its turn, seems pretty good in general.

• Decoupling the functions of running a tree and cleaning up the resources I think need to be provided in general.
• The ability to run a tree manually tick by tick also can be provided, although I would consider the cases. My reasoning is simply the logic when the manual tick is needed can be implemented inside the BT itself. But, it does not cost a lot to implement so I would also create it. Here can be a flag, but I would simply provide two methods on run:

fn run_with_cleanup(..)
fn run(..)

But those are just details.

• KeepRunningUntilFailure - well, I thought that can be expressed in BT itself if it is needed using hotree,
  something like(did not check, probably will require some changes in retry)

impl business_action(a:num);

sequence KeepRunningUntilFailure(tree_to_run:tree){
  retry invertor tree_to_run(..)
}

root main KeepRunningUntilFailure(business_action(a = 1)) 

If you think, it should be decorator, we can add it as well.
My thoughts regarding decorators were very simple:

• if it can be expressed in BT better to do it
• but we can also add them as soon as they are needed.

[samouwow]

Circling back to this issue as I think it should block part of #65. First some context.

The workaround I found for this issue was simply to not use the daemons at all. Since there were no daemons, clean up is trivial and doesn't take too much time. This actually turned out to be a huge win for me, as it meant I could reuse patterns from the rest of my codebase for the work the daemons would otherwise do. I now have a very strong preference to use Forester as a component of my application (e.g. as a library) rather than the core of my application (e.g. as a framework).

The link to #65 relates to preventing the delay decorator from blocking r_sequence, AKA delay returning running rather than blocking the whole thread with a sleep(). If delay no longer sleeps, then there will be no way to throttle long-running behaviour trees. This means anything that implements a delay decorator will consume 100% of the CPU while waiting for the delay to finish.

Now for the solution.

I'd like to propose we add three new functions to the API of forester_rs::runtime::forester::Forester:

1. tick() - which would:
   1. Create a TreeContext as a member variable of Forester if one didn't already exist.
   2. Run the while-loop of run_until() until we hit the root (e.g. whole tree errors, returns success or failure, or some node returns running).
   3. restart() the TreeContext if the root node finished (error/success/failure). See below for more on restart().
2. stop_all_daemons() - which would:
   1. Basically just call self.env.lock()?.stop_all_daemons()
3. restart() - which would:
   1. Delete and recreate the TreeContext.

run() would become a convenience function that calls tick() until completion, then calls stop_http() and stop_all_daemons(), while run_until() does the same with a limit on the number of times tick() is called. It would make sense to me for run() to take an Option<Duration> argument to throttle the CPU usage, but since this would break the existing API I'm happy to leave it out. Perhaps a spin duration could be set for run() in the Forester builder?

This design allows for:

1. Tree execution to be throttled, by reducing the frequency with which the user calls tick().
2. Forester to be easily run as a component of a larger application, since it will regularly feedback the running/success/failure status of the root node, without compromising its ability to work as a framework.
3. Trees to be looped without restarting the http / daemons if a user wants feedback, rather than looping with repeat(0) retry(0).

I'm very interested to get your thoughts on the above.

[besok]

This actually turned out to be a huge win for me, as it meant I could reuse patterns from the rest of my codebase for the work the daemons would otherwise do. I now have a very strong preference to use Forester as a component of my application (e.g. as a library) rather than the core of my application (e.g. as a framework).

Yeah, the thing was initially to decouple writing bt code from Rust and it is more like a framework approach rather than a library. In that paradigm, the daemons are vital to provide the background ops. But of course, having it as a library, you can handle all background processes independently as well.

If delay no longer sleeps, then there will be no way to throttle long-running behaviour trees. This means anything that implements a delay decorator will consume 100% of the CPU while waiting for the delay to finish.

Yeah, I agree, it does not look good. But the whole delay decorator is slightly our of the conception of the tree since it does not operate the real time.

The methods:

1 tick() - which would:

I agree, the method will enable to use Forester as a lib.

restart() the TreeContext if the root node finished (error/success/failure). See below for more on restart().

Will the restart lose the information about the current state? For instance, there's a m_sequence or running node, then it will drop the state or I just did not get this part?

stop_all_daemons() - which would

maybe rename to clean() or finalize() operation and stop the HTTP server also?

I recon, the idea is very strong and seems to be super useful.

[samouwow]

That's a very good point about restart() handling running nodes, and kind of ironic that I missed it given the amount of work I've just put into how the rest of the tree handles running nodes!

To do it properly I suspect a call to restart() should halt() any running nodes, so they can be in a fresh state for the restarted tree. All I was imagining was literally deleting and restarting the context, with the intention to drop the state for m_sequence etc, as you said.

Since I don't personally have any current need to restart() I'll just leave this out of the implementation. If someone else wants to implement it properly it can be done later. In the meantime, users can just call tick() or run() until the tree stops returning running.

Combined with your other suggestion that leaves two new functions:

1. tick() - which would:
   1. Create a TreeContext as a member variable of Forester if one didn't already exist.
   2. Run the while-loop of run_until() until we hit the root (e.g. whole tree errors, whole tree returns success or failure, or some node returns running).
   3. Delete the member variable TreeContext if the root node finished (error/success/failure). (No nodes are running so no need to halt())
2. finalize() - which would:
   1. Call self.env.lock()?.stop_all_daemons()
   2. Call stop_http()

run() and run_until() will be refactored to use tick() and finalize() under the hood but maintain the same external behaviour.

P.S. I've just realised we might have a running node if a reactive check returns error. I'll have to think more about the best way to handle that...

[besok]

That sounds very good to me.