Draft for python API

[rschilli-dev]

First off all thanks for your great work on another quite simple but accurate algorithm, I already worked with KISS(-slam) and its always a pleasure to work with those algorithms.

I am currently evaluating several approaches with an Ouster Os1 128 Lidar and a high-precision GNSS/INS-system and I loved the speedup factor off your offline approach in ROS. So I started to use your python API more intense for my setup and created a fork with some features/refactoring stuff I wanted to share. Since I also have experience with your other algorithms, I tried to orientate a bit on them, so here's a short overview:

• I've refactored the configuration part by using pydantic to simplify handling and validating the parameters through the pipeline. I know it brings a further dependency, but it really keeps the code simpler to handle and its orientated on KISS
• added an option to also define a TF-tree as yaml file (e.g. the result of a typical ros2 topic echo /tf_static ). The intention for me was, since I often work with "snipped sub-records" I often lose the tf_static topic and by increased complexity of the tree the current extrinsic_params could be painful to grab the correct values
• expand the logic for dumping deskewed scans by allow to define the desired file format (ply/pcd) as also added an option to dump the local_map. Therefore I track the accumulated local trajectory and dump the map every X meters (currently max_range / 2)
• the largest change: added native ouster support to process their raw UDP packets. I know this would not be an option for you, since I depend on the ouster-SDK therefore which brings kiss-icp with it in the latest version, but maybe its worth to take a look into it. Since my records are ros2 mcap that includes the osuter udp packets, its more an extension of the existing rosbag dataloader. But by using the SDK I can replay them as fast as possible which means in my case I have a 2-3x faster replay time in python, which comes close to the offline ROS node. It also supports using an external IMU topic instead of the internal one from ouster.
• last one is a minor improvement: start the visualization of rerun first AFTER everything was valid. Before rerun opened and then the algorithm crashed e.g. because of invalid TF ;-).

So, I just wanted to provide you some input for further enhancements. :) Since some of them are major changes, I didnt directly create a PR, moreover I do not have all supported datasets to ensure correct functionality for all of them.
Feel free to checkout my fork or give me feedback if you want to grab some of my changes as an PR :-)

[mehermvr]

Hi @rschilli-dev, thanks for all the suggestions and work on this! I'll have a closer look at your fork soon and I'll be more than glad to merge features in.

Unfortunately, I'm tied up with some other work for a few weeks and I cannot immediately get to this.
However, in the meantime if you'd like (and can), please open PRs targeting individual changes you've suggested. All 5 things you've said sound interesting to me. While a few of them do sound like very specific use cases, I don't mind having additional features as long as they seem easy to maintain (e.g., I will drop helipr support at some point because i don't think it'll actually be useful beyond academics).

Probably the native ouster support is the most extensive one, and before you make a PR for that and commit your time, i can first look at your fork to see if that's something I want to include here (though the answer is probably yes). I'll let you know as soon as i can.

The pydantic one is probably the one I might be least keen on (more dependencies). But if it does simplify the project and all the messy config boilerplate i have going now, I don't see why not.

I actually had to pause work on this project mid-refactoring, so apologies if things seemed a bit messy :)

P.S.: If you do open PRs, i have a local regression suite on some real datasets (too expensive for CI), so checking that won't be a problem.

[rschilli-dev]

Thanks for your feedback.
Sure, most of them were based on my use-case / dataset but since I have become familiar with other of your algorithms I then was seized by the desire to try implement the features cleanly, taking into account of some of your well-known approaches :-)

I will try to find the time to split them in more granular / independ PR, moreover I dont want to risk that my fork drifts too far away of your mainstream.

And for the ouster part: in kiss-icp you already have a dependency to the sdk, but an older one. In the meantime, Ouster integrated kiss to its own sdk as part of their mapping solutions, but therefore they pinned kiss-icp to an older one. So basicly, at the moment there could be some "circular" version conflicts when working with the latest versions.. Unfortunately Ouster only provide (for python) to integrate the whole sdk as a dependency, it would be "cleaner" if they would provide single modules (core / client), or you need to setup a complex cmake to clone and build only the desired modules for python.. So, maybe if this way of ouster integration would be interesting, the "cleanest" way could be to test it with an older sdk version in which their whole mapping (kiss-icp) is not included.

[mehermvr]

Im not too familiar with the ouster sdk, but i wouldn't add kiss-icp as a dependency to this, even as optional. Kiss is a bit too broad for this purpose alone, and while i dont mind more dependencies per se, i'd like each of them to have a very specific and minimal purpose if possible.
Given the situation with the ouster sdk you explained, its probably not worth it to consider that as a PR here.

I've also thought about the deskewed scan dumping extension you talked about. And on second thought, that is probably too specific of a feature as well. Dumping the local map overlaps with the ability to dump the scans (as a user can post process to get the map they want). Since this is not a mapping package in the first place, i probably wouldnt add more features for that purpose. The deskewed scan dumping itself is a relatively minimal addition since its essentially one additional function call.