Skip to content

Conversation

@4lon
Copy link

@4lon 4lon commented Nov 13, 2025

Hello!

While working on NixOS I was running into compilation issues with this sdk. I narrowed it down to this clamping function which seems to have some sort of templating issue leading to compilation failure.

I believe this updated clamping function should be equivalent and more clearly operating element wise but I have not had the chance to validate this, only to confirm that it now builds on NixOS and still builds in an Ubuntu docker image.

My main use for this sdk is within the ouster_ros library. I intend to propagate this change into the ouster_ros repo (specifically the branches that no longer use this repo as a submodule like rolling-devel) but I thought that this would be the best place to get this update validated.

Templating issues steming from this clamping function were causing
compilation errors. More verbose element wise clamping fixes this issue.
@Samahu
Copy link
Collaborator

Samahu commented Nov 13, 2025

Another user reported the same problem but check out the responses and specifically this comment #673 (comment) which suggests the use of an supported version of Eigen.

@Samahu Samahu self-assigned this Nov 13, 2025
@4lon
Copy link
Author

4lon commented Nov 14, 2025

Ah, thank you for pointing this comment out! Apologies I didn't see this comment when I was looking for similar issues.

You're right, building against stable eigen 3.4.0 did resolve this issue. However I also tried it against eigen 3.4.1 as the original comment was using and also experienced build failure. I did this originally on my Nix machine however I put together this dockerfile as an example that can be used to test whether this library can build against eigen 3.4.1

FROM ubuntu:24.04

RUN apt -y update && \
    apt -y --no-install-recommends install \
    git \
    ca-certificates \
    cmake \
    gcc \
    build-essential \
    libcurlpp-dev \
    libpcap-dev \
    libtins-dev \
    zlib1g-dev \
    libpng-dev \
    libflatbuffers-dev \
    libgl1-mesa-dev \
    libglfw3-dev \
    libtbb-dev \
    robin-map-dev

### Use the following two sections to switch between building the sdk against 3.4.1 and 3.4.0
## Eigen 3.4.1 build requirements
RUN apt -y update && \
    apt -y --no-install-recommends install \
    wget \
    libunilog-dev

RUN wget https://gitlab.com/libeigen/eigen/-/archive/3.4.1/eigen-3.4.1.tar.gz
RUN tar -xf eigen-3.4.1.tar.gz
WORKDIR eigen-3.4.1/build
RUN cmake ../
RUN make install
WORKDIR /

RUN wget https://github.com/ceres-solver/ceres-solver/archive/refs/tags/2.2.0.tar.gz
RUN tar -xf 2.2.0.tar.gz
WORKDIR ceres-solver-2.2.0/build
RUN cmake ../
RUN make install
WORKDIR /

### Eigen 3.4.0 build requirements
# RUN apt -y update && \
#     apt -y --no-install-recommends install \
#     libceres-dev \
#     libeigen3-dev 

#### Use the following two sections to switch between ouster-sdk/master and proposed changes
### Ouster sdk master branch
RUN git clone https://github.com/ouster-lidar/ouster-sdk.git
### Proposed changes
# RUN git clone -b change-clamping-function https://github.com/4lon/ouster-sdk.git

WORKDIR ouster-sdk/build
RUN cmake ../
RUN make

Unaltered, this will build the ouster sdk against eigen 3.4.1 (this is quite slow as it also requires rebuilding ceres for version matching since ceres also depends on eigen). This build unfortunately fails.

You can swap the commented out git clone at the bottom to try the proposed changes, and see that it now builds. This also still builds when using eigen 3.4.0, as can be confirmed by swapping the first sections.

I know this isn't a fool proof validation of the changes but I do think it warrants some consideration as it does seem like when eigen is updated to 3.4.1 on more distributions, this sdk may fail to build because of the current clamping function and I don't think it is just a byproduct of a specific eigen setup.

Copy link
Collaborator

@Samahu Samahu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I am able to re-produce the problem and I don't see a problem with the switching to the explicit cwise Min/Max. Approved!

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