Skip to content
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

Cannot compile sophus_pybind in Ubuntu 20.04 #537

Closed
edubya opened this issue May 16, 2024 · 6 comments
Closed

Cannot compile sophus_pybind in Ubuntu 20.04 #537

edubya opened this issue May 16, 2024 · 6 comments
Labels

Comments

@edubya
Copy link

edubya commented May 16, 2024

Issue

I can't seem to compile sophus_pybind module in Ubuntu 20.04 without setting the following compiler flags:

-Wno-error=unused-parameter
-Wno-error=sign-compare
-Wno-error=maybe-uninitialized

To Reproduce

  1. Clone repo into clean ubuntu-20.04 install (or using docker Dockerfile and build script below)
  2. Do any of the following: pip install . or python setup.py install or mkdir build && cd build && cmake .. -DBUILD_PYTHON_BINDINGS=ON && make

Expected behavior

sophus_pybind module compiles and can be used in Python.

Sophus version: commit hash

d270df2

Additional context

-Werror=unused-parameter is caused by small typo in line 47 of SE3PyBind.h, which should be bool load(handle src, bool) {. See also SO3PyBind.h.


-Wno-error=maybe-uninitialized are thrown by Eigen 3.4.0 code. Since these errors go away after removing the -O3 flag from CMAKE_CXX_FLAGS_RELEASE in CMakeLists.txt the warnings are likely wrong. Maybe different optimization flags need to be set.


-Werror=sign-compare are all caused by comparisons of size_t integers to Eigen::Index in every for loop in SO3PyBind.h and SE3PyBind.h. For example:

SO3PyBind.h:89:30: error: comparison of integer expressions of different signedness: 'size_t' {aka 'long unsigned int'} and 'Eigen::Index' {aka 'long int'} [-Werror=sign-compare]
   89 |         for (size_t i = 0; i < rotvecs.rows(); ++i) {

Docker

After cloning Sophus into local directory Sophus, build new docker image using:

Dockerfile

FROM ubuntu:20.04 as base
ENV DEBIAN_FRONTEND="noninteractive" TZ="America/New_York"
WORKDIR /build

ADD ./setup_docker.sh /build
ADD ./Sophus /build/Sophus
RUN /bin/bash -e /build/setup_docker.sh

setup_docker.sh

#!/bin/bash

apt-get update -y
apt-get upgrade
# apt install -y libunwind-dev
apt-get install cmake git -y

apt-get install libpython3-dev python3-pip -y
pip3 install numpy pytest

apt-get install gfortran libc++-dev libgoogle-glog-dev libatlas-base-dev libsuitesparse-dev libceres-dev ccache

git config --global advice.detachedHead false
git clone --branch 3.4.0 https://gitlab.com/libeigen/eigen.git
cd eigen && mkdir build && cd build && cmake ..
make
make install

cd ../..
git clone https://ceres-solver.googlesource.com/ceres-solver ceres-solver
cd ceres-solver
mkdir build
cd build
ccache -s
cmake -DCMAKE_CXX_COMPILER_LAUNCHER=ccache ..
make -j8
make install

git clone https://github.com/fmtlib/fmt.git
cd fmt
#git checkout 5.3.0
mkdir build
cd build
cmake -DCMAKE_CXX_COMPILER_LAUNCHER=ccache ..
make -j8
make install

cd ../../Sophus
python setup.py install --verbose
@edubya edubya added the bug label May 16, 2024
@strasdat
Copy link
Owner

strasdat commented Jun 9, 2024

Yeah, it seems like the CI action is broken and possibly never worked since it was configured for the "main" branch, though the default branch used to be called "main-1.x" until today. Unfortunately I overlooked that when merging #522.

Now we do have failing CI:

https://github.com/strasdat/Sophus/actions/runs/9434646626

@strasdat
Copy link
Owner

strasdat commented Jun 9, 2024

I'm not a user of the py-bindings and overall have very little knowledge of this are of the repository, possibly @chpeng-fb can help, who is the author of #522.

Overall, a good first step would be to repair the CI action (but not necessarily have it pass), or maybe just get more minimal pybind CI job in place. @edubya or @chpeng-fb , could you help with that?

Once, we have running CI, I can hopefully help with c++ / build flags related errors.

@chpeng-fb
Copy link

@edubya Thank you for reporting the issue and pin point the issue. I will fix this issue soon.

@strasdat
Copy link
Owner

@chpeng-fb, I just merged a larger maintenance PR #538. It should not have significant impact on the py-bindings but if it does or you have any other questions, please reach out!

@chpeng-fb
Copy link

Thank you for the update. I will test my fix again after your merge.

@strasdat
Copy link
Owner

Thanks to @cybaol, CI is fixed, as well as the original issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants