Skip to content

LOAM#38

Merged
contagon merged 7 commits into
masterfrom
feature/loam
Jul 17, 2025
Merged

LOAM#38
contagon merged 7 commits into
masterfrom
feature/loam

Conversation

@DanMcGann
Copy link
Copy Markdown
Collaborator

This PR adds a LiDAR-Only LOAM pipeline to evalio!

It makes use of my LOAM implementation from: https://github.com/DanMcGann/loam

For this implementation to work within evalio we do have to implement some logic within the binding code.

Namely, logic for maintaining a local map. The original LOAM had its own custom local mapping code that probably wont work great modern LiDAR scan sizes. Instead I opt to implement a relatively simply local map that consists of the last K scans which are transformed into a common frame (the most recently registered scan's frame). K is made available as a configuration parameter so that users could also choose to run scan-to-scan if desired.

@DanMcGann DanMcGann requested a review from contagon July 9, 2025 23:43
@DanMcGann
Copy link
Copy Markdown
Collaborator Author

hmmm clang is not happy.
My local version is: clang-format version 10.0.0-4ubuntu1
In my experience clang is not particularly version dependent.
Instead I wonder if I modified my defaults at some point, or if there is a sneaky .clang-format hiding in a parent directory on my machine that clang is picking up and using the config from.

Either way it may be good more broadly to add a .clang-format to the project so that local config is not likely to affect formatting and in-turn the CI checks?

@contagon
Copy link
Copy Markdown
Owner

We definitely should have a clang-format file. I'll open another PR with it, as some of the clang defaults drive me nuts:) I would guess minor versions of clang don't make a huge difference, but the CI is running clang-format 20 which is quite a jump. If you're cool with it, I can format and push it for you.

It also looks like the CI build isn't finding your LOAM directory for some reason, see failing to find it and not showing up in the ls command. This is really strange, as it looks like it should've been cloned fine earlier in that pipeline...

There's also a couple of things that I'd like done that we can do either via a patch, or if you'd like to add it directly to the LOAM codebase.

  1. In the LOAM CMake file, it'd be good to add a version to the project command. evalio will snag this for versioning pipelines (as different results can happen across versions).
  2. I'd much prefer to pull all of our dependencies from vcpkg rather than fetch content... as I'm sure other pipelines will use Ceres/nanoflann as well and we don't need to be building multiple versions of them. I think adding FIND_PACKAGE_ARGS to the FetchContent_Declare would default to a finding system dep before pulling or it can be switched to find_package in a patch.

Let me know if you'd like to do these in a patch, or in the LOAM codebase. I'd be happy to play around with getting it working, as I've done it before and have all the vcpkg stuff setup.

On the code side of things - looks great. I made a small request for some docs in one spot, but everything else look sgood.

Comment thread cpp/bindings/pipelines/bindings.h Outdated
This enables the loam implementation to better support vcpkg and avoid
fetch content to grab dependencies.
@DanMcGann
Copy link
Copy Markdown
Collaborator Author

Awesome Thanks for the feedback easton!

  1. Doc string as been added to the python bindings.

  2. LOAM version was added to the loam repo. Now we explicitly grab v1.0.0.

  3. To support VCPKG I added a patch file to add the required lines to the fetch content calls. I decided this was preferred for now, as I would like the LOAM repo to continue to support Ubuntu 20.04 defaults (older cmake). What can I say roboticists are slow even with EOL distributions ¯_(ツ)_/¯.

  4. I think I identified the issue with finding the LOAM folder. After LIO-SAM there was no cd back to the top level directory, so I bet it was cloning into the LIO-SAM directory. This as been fixed now:

@DanMcGann
Copy link
Copy Markdown
Collaborator Author

CI is failing due to an issue at the intersection of vcpkg and Ceres.

Looks like Ceres wants the GLOG package, but vcpkg is not detecting and installing it as a dependency?
Happy to try and run this down, but @contagon may know a sollution from your vcpkg experience

@contagon
Copy link
Copy Markdown
Owner

Just made some pushes that seemed to have helped. I switched the patch to just use find_package, just so it's not accidentally pulling anything, and I added in Ceres and nanoflann to vcpkg dependencies. (also got clang-format to behave)

It looks like now it's hitting some issues with default templates - I think they just need to be removed from the -inl.h files. I vaguely remember hitting this last time I was using loam with the lidar evaluations stuff (which reminds me I might have some PRs for that...).

@DanMcGann
Copy link
Copy Markdown
Collaborator Author

Fixed in the LOAM repo!
Fun fact this is an effect of a bug in GCC from 2011 that was not address until 2022!
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50370

@DanMcGann
Copy link
Copy Markdown
Collaborator Author

Gad you figured this out!
I spent some time poking around and could not deduce what exactly cibuildwheel was doing.
Thanks for the help, and apologies I couldn't find a fix.

@contagon
Copy link
Copy Markdown
Owner

No sweat - it ended up just needing to build loam as a static not shared library. I managed to hit it on my local machine, so wasn't too hard to debug.

@contagon contagon merged commit 6bda93f into master Jul 17, 2025
12 checks passed
@contagon contagon deleted the feature/loam branch July 18, 2025 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants