several issues using the correct executor #307
Comments
|
Hmm... we run this basic launch file all the time with no problems. If you want no adaptation then take out fremen from top nap and add the following instead: |
|
So, assuming you're running this launch file, what's going wrong? |
|
for starters, we are not sure which task_executor to actually launch ;-) So, if you could tell us which one to use, then we say what's not working with it. @francescodelduchetto can say more about it, I'm sure, but if I recall, launching the MDP one the first hurdle was that it requested some door config... |
|
Run |
|
Yes, the I just tested it and got this error/warning fo the doors and yet it seems to schedule and start demanded tasks properly. |
|
Yep, it's a bit cryptic, but |
|
Hooray, sorry for bothering you @hawesie I thought we tried that one and it wouldn't run anything... So, that's the recommended one. Perfect. Thanks. |
|
We are still struggling with it, sorry. Whenever we submit a task, it ends with "Adding Doors" and then hangs, and also other services we call never return. There is an |
|
The mdp executor actually remains hung every time here: that service call never returns. It's odd how it sometimes works (like 3 days ago) and others not. Any idea on why this is happening? |
|
Could it be that Fremen is crashing and not providing predictions? Sorry away from computer so can't look in detail right now |
|
If you don't have the door_prediction service at |
|
Thanks, @bfalacerda that was our suspicion. Still a bit of a mystery that it looked as if it worked before, but I suspect that was just not running the correct version or something. |
|
I just noticed that it should work. You should get the following warning: when the door prediction is called, but it shouldn't hang. Maybe it's the edge prediction that died, as @hawesie suggested? |
|
That might explain it working before. Sometimes a new (corrupted) nav_stat makes some fremen prediction node die. When that happens the prediction service remains available, so the executor can call it, but then it hangs there, because it doesn't get a response. If these prediction services (for edges and doors) aren't provided at all, the executor will spit out warning but it will not hang (contrary to what I said in my first comment). |
|
I tried the edge prediction and it responded... as it should. But I agree, let's cut another unreliable piece out of the equation. @hawesie already mentions this: Is that the only thing to start? Do I start it in addition or instead of what? Instead of this: I'm sure you have a simple test script without fremen etc that can serve as a template for us, right? Somehow, I fail to find it though. |
|
@marc-hanheide did you tried edge prediction on your notebook or on the robot? Because on the robot is not working. I will describe here what happens given I don't know too much abotu fremen. This is what happen on the robot:
...and then the error number one again and fremen restarts. |
|
Hah! I checked it on the robot, but I probably checked the wrong one! I hate it when @hawesie is correct ;-) OK, then fremen appears to be the problem indeed. We should cut it out for now. Maybe @Jailander can help with this then. Indeed, we should for now not use fremen, but try the solution @hawesie mentioned above. @bfalacerda it would still be good to make the door-stuff more optional, if possible, please. But @francescodelduchetto, then I suppose it is related to your new way of disabling some nodes temporarily, where indeed fremen might be in some undefined state. That said, in @francescodelduchetto's use case, we need to "ban" some nodes for navigation temporarily. But I suppose the MDP is assuming a static topo map, right? Can it deal with a changing topological map @bfalacerda ? |
|
@marc-hanheide it can deal with topo map changes, it subscribes to /topological_map and updates the MDP when something is published there. we needed this for AAF if I remember correctly. |
|
Hi I just saw this, will take a look tomorrow, it could be a problem on the stats, it looks like there is something weird there |
|
@marc-hanheide The errors happen also with the upstream code without my modifications, so it is not caused by that. But, indeed what I added may still cause other problems to fremen (for example I can imagine that some of the estimates will be wrong when some nodes are banned). |
|
And my modification now does not change the topological map, it just ignores the banned nodes when planning the topological path. However, if fremen supports changes in the map it may be worth modifying the map online? So that the predictions and statistics should be more accurate...right? |
|
If you want certain nodes not to be traversed during task execution then those should be part of the task. That's exactly what our planning system is built for. We may need to extend the interface a bit, but our task specification language supports nice exclusions naturally. We also built blacklisting into the routine node due to some annoyed office workers. |
|
@hawesie that's awesome, I was trying to reinvent the wheel then. Is there some documentation on how to exclude and blacklist nodes? |
|
@francescodelduchetto currently excluding tasks is only possible if you use LTL tasks directly, which may not be convenient for you. We can find a way to make it easy though, e.g. by you providing a service which lists excluded nodes. The blacklisting is only available if you manage tasks by the routine (https://github.com/strands-project/strands_executive_behaviours/blob/hydro-devel/routine_behaviours/src/routine_behaviours/robot_routine.py). Maybe we could have a quick call to discuss this... http://doodle.com/nickhawes |
|
Hi @francescodelduchetto @marc-hanheide , the problem is that the Fremen server crashes every time, I will take a look at it but hope @gestom sees this message too ;) |
|
Another thing I notice by using the MDP navigation is that: sometimes the path choosen are not seemingly optimal (taking long paths to reach the goal) and sometimes the robot loops between 3 nodes without reaching the goal. |
|
The planner is using the straight line distance between nodes as the cost function to minimise. Can you paste a screenshot of the topological map? |
|
This is the entire topological map This is the focus on the waypoints 16, 17 and 18 where the robot was looping |
|
The MDP planner assumes that two nodes are connected in the topological map iff the robot can navigate between the two nodes without visiting any other node. You have many edges here that don't satisfy this property |
|
Oh, yes, @francescodelduchetto those edges should be removed. |
|
Ok, I cleaned some edges. Does it looks good to you? I will let you know if the looping still happens |
|
yes, it looks good. |
|
@francescodelduchetto don't forget to delete the |
|
Also just out of curiosity what action is that red edge to the right? |
|
@Jailander oh nothing is just that I didn't assigned any action to that, but now it is black as the rest;) Strangely I didn't get any error from fremen without removing the nav stats... |
|
Ok, but you should remove them just to be safe. You can do it from
|
|
Hi @hawesie, I just deployed on the robot the last version of I tried passing |
|
Yikes, I must have merged badly. I’ll look into this later this week.
|
|
@bfalacerda is the above parameter needed with your updated code? I notice that this line looks similar in purpose to the old parameter https://github.com/strands-project/strands_executive/blob/kinetic-devel/mdp_plan_exec/scripts/mdp_robot_policy_executor.py#L97 |
|
I don't see any old parameter on that link, is it correct? |
|
But the line you linked first is just me setting the server as aborted when there is no policy, was that what you intended to link? Looking at the code, I think it's needed: https://github.com/strands-project/strands_executive/blob/kinetic-devel/mdp_plan_exec/scripts/mdp_robot_policy_executor.py#L101 The boolean variable linked above ensures the robot does a navigation action before it executes any other action. |
|
But what does |
|
it ensures ensures the robot does a navigation action before it executes any other action, i.e., exactly the opposite of what is intended. I don't understand exactly what is old and what is new code. The most up to date code in terms of the logic for navigating (or not) before executing an action is the one in https://github.com/strands-project/strands_executive/blob/deprecated-kinetic-devel/mdp_plan_exec/scripts/mdp_policy_executor_extended.py#L38 |
|
I didnt merge the param thing to the fork we were using, that might be why this is confusing. |
|
I'm a bit lost. Let's chat in person. |
|
@francescodelduchetto is this still an open issue for you? We haven't provided any kind of fix, but before I start trying to puzzle through the messages I wanted to check you still cared. |
|
Hi @hawesie, yes it is still an open issue. We are using my fork of this repo for the robot now, but I guess it would be nice if we can return to the upstream one |
|
I guess that the main problem and the reason I started to use my fork is what described in here #307 (comment) . |
|
The |
|
Thanks, that's super useful. So the task for me is to put back the |
|
Yes, I think so. Thank you! |
|
Ok, @bfalacerda is now in charge of this. It's quite a hard thing to change directly, so this probably won't happen immediately. |
|
@francescodelduchetto please check #319. Should be back to the behaviour you asked for. |
@francescodelduchetto and I are a bit lost what the "correct" executor/scheduler would be to use for our museum case.
The problem: We have so far tried to use https://github.com/strands-project/strands_executive/blob/kinetic-devel/task_executor/launch/task-scheduler-top.launch as a simple scheduler. However, that one seems to be deprecated, and lacking recent features (e.g. time critical tasks). Also with that one, we had the following issue:
@francescodelduchetto submitted in private issue https://github.com/LCAS/LindIMP/issues/40:
So, maybe we should be using the MDP scheduler? That one, however, in the current version uses the a door config file:
strands_executive/task_executor/launch/mdp-executor.launch
Lines 9 to 11 in 74d4d03
And it doesn't seem to work properly. Can @bfalacerda or @hawesie suggest what the correct launch file would be for a "simple" (no doors, no adaptation, no bells and whistles), but robust and full-featured scheduling? I feel we are doing something stupid here. If it helps, I can invite you guys to the private repo to understand our situation better. Simple first questions: Which launch file should we be using?
The text was updated successfully, but these errors were encountered: