-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved handling of multiple trajectories and bags in the offline node #4
Changes from 4 commits
0fd32b6
350d5fd
22ee1f9
6880769
d7c8817
94d5d84
daaaeb8
aa44390
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
# Improved handling of multiple trajectories and bags in the offline node | ||
|
||
## Summary | ||
[summary]: #summary | ||
|
||
The offline node currently supports multiple sequential (non-concurrent) trajectories via loading multiple bags, where each bag is a separate trajectory. | ||
This RFC aims to give the user greater flexibility with the offline node when dealing with multiple robots, multiple (possibly concurrent) trajectories and multiple bags. | ||
|
||
## Motivation | ||
[motivation]: #motivation | ||
|
||
The online node currently supports tracking multiple concurrent trajectories in real time. | ||
This enables the Cartographer user to do cooperative SLAM using multiple robots. | ||
However, support for this in the offline node is currently limited - only a single robot configuration may be used, and the trajectories cannot be concurrent. | ||
Since this functionality is supported in the online node, this RFC aims to expand the offline node to be able to provide the same functionality, but at increased speed. | ||
|
||
For example, a user has bagged a swarm of robots and wants to try out cooperative SLAM with Cartographer. | ||
They are interested in crunching the bags as fast as possible. | ||
Perhaps they have a bag for each robot, perhaps they have all robots in a single bag - the offline node should have a flexible configuration interface to allow for various possibilities. | ||
|
||
The concurrent trajectories could be processed either sequentially or in parallel - it should not influence the end result in any way, since local SLAM is not influenced by other trajectories, and global SLAM should arrive at the same pose graph at the end, no matter what the order of insertion is. | ||
However, processing and displaying the concurrent trajectories in parallel, like the online node can do, would be more useful and increase the coolness factor. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could also be handled by a rosbag filter or a short python script. Essentially translate each There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To clarify: that means that the offline node could just merge sort from multiple bags: always process the newest message and push it through the pipeline. It would still work exactly the same in the current use case (just relaxing that the bags must be provided in the same time order), but would automagically support playing bags with overlapping timestamps. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@SirVer That's right! So there is no need to remap time in the bags, right? I agree with you on the rest (user can split one bag into multiple bags, or join multiple bags into one, so that in the end, the data obeys 1 trajectory : 1 bag format). I think you misunderstood the text in the RFC: the user should never remap time in the bags. The offline node should accept both simultaneous and sequential trajectories/bags. @wohe suggested that we could process simultaneous trajectories sequentially, to which I responded that it would be better if we processed them truly simultaneously like you described, by merge sorting them (I called this collating, was that wrong? Better change it to merge-sort.).
You mean the oldest? :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Only if you want to have the bags from one robot (i.e. the deutsches museum runs) looking like they were recorded at the same time. You know, for the coolness factor.
I think collating is correct as well - merge sorting is the means to get collated data here.
yes, sure. Will you update the RFC with these new information? |
||
|
||
|
||
### Overview of possible dataset scenarios | ||
Let's give an overview of all possibilities how a dataset layout and content could look like: | ||
|
||
| Robot plurality | Trajectory plurality | Bag plurality | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the goal here is to support |
||
|-------------------------------------------------------------------------|------------------------------------------------------|------------------------------------------------------------------------| | ||
| __A.__ Single robot | __1.__ Single trajectory | __a.__ Single bag | | ||
| __B.__ Multiple identical robots (using the same sensor topics/frames) | __2.__ Multiple trajectories, necessarily sequential | __b.__ Multiple continuous bags (like a single bag) - a bag aggregate | | ||
| __C.__ Multiple different robots (different sensor topics/frames) | __3.__ Multiple trajectories, possibly concurrent | __c.__ Multiple bags | | ||
|
||
The online node currently supports the following combinations: | ||
__A1__, __A/B 2__ and __C 2/3__ (using `StartTrajectory` and `FinalizeTrajectory` service calls) | ||
|
||
The offline node currently supports: __A1a__, __A/B 2c__. | ||
|
||
The online node can never support __B3__, because only single robot's data can be published on a topic at a time. | ||
Also, __B3__ and __a/b__ is impossible for the same reasons. | ||
|
||
Note that some combinations are invalid, such as __A3__ (a single robot can't be on multiple concurrent trajectories), and __B/C 1__ (there can't be a single trajectory if there are multiple robots). | ||
Furthermore, __A/B 2 a/b__ would require us to be able to specify when a trajectory stops and begins within the same bag aggregate and on the same sensor data topics, which would be too complicated and out of scope. | ||
|
||
The aim of the RFC is to enable the user to use the offline node for __C2/3__, like the online node; to preserve the ability of doing __A/B 2c__; and, to possibly offer additional functionality. For example, __b__ (bag aggregates), and __B3c__, which would otherwise be impossible with the online node. | ||
|
||
## Approach | ||
[approach]: #approach | ||
|
||
The proposed offline node configuration for multiple, possibly concurrent trajectories (__3__) should closely follow the way of the online node for achieving this, listed here for reference: | ||
- Performing multiple calls of the `StartTrajectory` service, with possibly different `TrajectoryOptions` and necessarily different sensor data topics (__C__); all is specified in the service request. | ||
- Alternatively, the `start_trajectory` helper node is invoked multiple times, with possibly different `.lua` files for `TrajectoryOptions` and necessarily different sensor data topics (__C__); the recently merged PR googlecartographer/cartographer_ros#584 enables specifying different topics via topic remapping. | ||
- Specially, if a trajectory is finalized, we may start another independent trajectory which uses the same sensor data topics and possibly different `TrajectoryOptions` (__B2__). | ||
This is already achievable in the offline node by simply loading multiple bags, which creates a trajectory for each bag, although `TrajectoryOptions` are necessarily the same for all trajectories (__B2c__). | ||
The ability to do this has to be preserved or expanded. | ||
|
||
If we suppose that there is a mandated 1:1 relationship between bags and trajectories in the offline node, the following configuration can be proposed. | ||
|
||
### Configuration #1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would advise to keep the design and scope limited to what is absolutely necessary to solve your usecase. Sure it would be nice to have the additional functionality of options 2 and 3, but with some bag surgery it seems you can reduce both of these to option 1. I would suggest to move ahead with that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1. All functionality needs to be taught somehow. Cartographer is already a complex system, so adding more features will make it even harder to understand. Multi trajectory is not a self evident concept in general, so making it as simple to understand as possible is important. My suggestion: Move this explanation to discussion points, explain why handling one trajectory per bag is sufficient with some bag filtering and that we limited the scope to this, then simplify the rest of the rfc to only deal with this use case. |
||
|
||
__For each bag (trajectory)__, the configuration is a __tuple__ consisting of: | ||
- `TrajectoryOptions` (given in a `.lua` file) | ||
- A set of sensor data topics | ||
- Boolean `use_bag_transforms` which allows us to ignore transforms in the bag | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would this work with multiple bags setting use_bag_transforms to true? Couldn't the frame names clash and how do you want to deal with that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each bag (or |
||
- An optional `.urdf` file. | ||
|
||
Both `TrajectoryOptions` and the sensor data topics can be identical for multiple different bags (preserving __B2c__). However, in addition to that, the trajectories can be concurrent (__B3c__). | ||
|
||
Because the trajectories may be concurrent, due care would need to be taken to ensure that data published on the same topic, but in different bags, gets routed to the appropriate trajectory. | ||
Also, each trajectory should have its own transform buffer, to avoid interference between different bags. | ||
- For example, we may have two identical separate robots driving concurrently (__B3__); all the frames and topics are identical for both robots. | ||
The online node cannot handle this (we would have to ensure that the sensor data topics would be different by e.g. using different namespaces for each robot, as well as having different frames; conflicting transforms with same frames would be an unsolvable nightmare). | ||
- However, if we had a separate bag for each robot (__B3c__), the offline node would have the advantage of knowing which data originated from which bag, so it could handle this case. | ||
|
||
Going further, allowing multiple trajectories in a single bag (__C 2/3 a__) and relaxing the 1 trajectory : 1 bag restriction would mean that the configuration of each bag becomes a set of tuples mentioned above. | ||
This is given in Configuration #2. | ||
|
||
### Configuration #2 | ||
|
||
Similar to Configuration #1, but have a __set of tuples for each bag__. | ||
Each tuple describes a trajectory in the bag. | ||
|
||
For both Configuration #1 and Configuration #2, all concurrent trajectories (whether in the same bag as permitted by Configuration #2, or in different bags) are __collated__. | ||
|
||
### Handling split bags | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Joining bags can be done by a script too. Let's not rebuild features that ROS tools already give us. I am fine with adding some scripts (to be outlined here) to cartographer_ros that help with joining and reshuffling timestamps for bags. This will also allow to drop the concept of a BagAggregate - which seems an unnecessary complexity in this RFC. |
||
|
||
Another common use case is (__b__) - a single continuous trajectory is split into multiple bags (e.g. when using the split option of `rosbag record`). | ||
|
||
This could be handled by defining a `BagAggregate`: a list of one or more continuous bags, which are to be treated as one bag. | ||
|
||
### Configuration #3 | ||
Configuration is given as a __set of tuples for each `BagAggregate`__, each tuple describing a trajectory in the `BagAggregate`. | ||
|
||
As before, any concurrent trajectories are collated. | ||
Each `BagAggregate` has its own transform buffer (which is shared by all bags in the aggregate). | ||
|
||
### Data format of the configuration | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section is most interesting to me - but it needs to be written. For example would we replace the current command line options or add a new one that reads a lua file as input? How would such a Lua file look? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #helpneeded :D I am currently a bit short on time, but I'll get back to this RFC when I can. |
||
|
||
The configuration can be given in a `.lua` file, similar to `assets_writer`. | ||
|
||
(Full specification missing. #helpneeded) | ||
|
||
## Discussion Points | ||
[discussion]: #discussion | ||
|
||
Should Configuration #1, #2 or #3 be used? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree with this still. Why should we support one bag - multiple robots? That is just an
rosbag filter
away from a sane input to cartographer_ros. It also deals better with the problem of how to replay TF messages in the bag - if you have one per robot, they are namespaced to the robot.