From 21dce0f4bacac549d15d31c099958084d4087a79 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Fri, 13 Dec 2024 16:47:35 -0600 Subject: [PATCH 01/15] Use a consistent ubuntu across all Linux CI jobs. Rollout of ubuntu-24.04 as ubuntu-latest broke our Python CI due to newer and stricter GCC. Mitigate by using ubuntu-20.04, matching what we currently use for the PyPI release. --- .github/workflows/ci_python.yml | 2 +- .github/workflows/docs.yml | 2 +- .github/workflows/format.yml | 2 +- .github/workflows/pypi-release.yml | 4 ++-- .github/workflows/release.yml | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci_python.yml b/.github/workflows/ci_python.yml index fed1494b..00f0c608 100644 --- a/.github/workflows/ci_python.yml +++ b/.github/workflows/ci_python.yml @@ -18,7 +18,7 @@ jobs: # 1. they removed support for python 3.8 & 3.9 on macos-14 (now macos-latest) # - See: https://github.com/actions/setup-python/issues/696#issuecomment-2071769156 # 2. `test_pose` fails on macos-13 and macos-14 (but not macos-12) - os: [ubuntu-latest, macos-12] + os: [ubuntu-20.04, macos-12] python-version: ['3.8', '3.9', '3.10', '3.11', '3.12'] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index c92ad7b9..25f92b1c 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -12,7 +12,7 @@ on: jobs: deploy: name: Deploy to GitHub Pages - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v2 with: diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index 7a916a0f..af3e150b 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -14,7 +14,7 @@ concurrency: jobs: pre-commit-check: - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v3 - uses: actions/setup-python@v3 diff --git a/.github/workflows/pypi-release.yml b/.github/workflows/pypi-release.yml index ab4b6ac0..0db671da 100644 --- a/.github/workflows/pypi-release.yml +++ b/.github/workflows/pypi-release.yml @@ -41,7 +41,7 @@ jobs: build_sdist: name: Build source distribution - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v4 - name: Build sdist @@ -52,7 +52,7 @@ jobs: upload_pypi: needs: [build_wheels, build_sdist] - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 if: github.event_name == 'release' && github.event.action == 'published' steps: - uses: actions/download-artifact@v3 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 798ca7db..a67f9c91 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,7 +13,7 @@ jobs: # Based on https://dev.to/eugenebabichenko/automated-multi-platform-releases-with-github-actions-1abg create_release: name: Create Release and Provide Upload URL. - runs-on: ubuntu-latest # OS to create release, not for build + runs-on: ubuntu-20.04 # OS to create release, not for build outputs: # This job will provide URL for build jobs to use for uploading assets upload_url: ${{ steps.create_release.outputs.upload_url }} From c95dc7eb639f7a684c13175a5e1383bba7d4a105 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Mon, 16 Dec 2024 14:00:45 -0600 Subject: [PATCH 02/15] Bump ubuntu and macos runner versions across CI. Use supported runners that still support Python 3.8, matching the system Python on the Brain. --- .github/workflows/api_reference.yml | 2 +- .github/workflows/ci.yml | 8 ++++---- .github/workflows/ci_python.yml | 9 ++++----- .github/workflows/docs.yml | 2 +- .github/workflows/format.yml | 2 +- .github/workflows/pypi-release.yml | 6 +++--- .github/workflows/release.yml | 6 +++--- .github/workflows/rs.yml | 4 ++-- 8 files changed, 19 insertions(+), 20 deletions(-) diff --git a/.github/workflows/api_reference.yml b/.github/workflows/api_reference.yml index 9bd24108..ca54ddc3 100644 --- a/.github/workflows/api_reference.yml +++ b/.github/workflows/api_reference.yml @@ -12,7 +12,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-20.04] + os: [ubuntu-22.04] fail-fast: false diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 42242687..79861a65 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,13 +20,13 @@ jobs: strategy: matrix: include: - - os: ubuntu-20.04 + - os: ubuntu-22.04 compiler: gcc asan: off - - os: ubuntu-20.04 + - os: ubuntu-22.04 compiler: clang asan: off - - os: ubuntu-20.04 + - os: ubuntu-22.04 compiler: clang asan: on @@ -41,7 +41,7 @@ jobs: working-directory: ./scripts run: | ./install_deps_ubuntu.sh - if: matrix.os == 'ubuntu-20.04' + if: matrix.os == 'ubuntu-22.04' - name: Download dependencies (proto) run: | diff --git a/.github/workflows/ci_python.yml b/.github/workflows/ci_python.yml index 00f0c608..84f6dcd9 100644 --- a/.github/workflows/ci_python.yml +++ b/.github/workflows/ci_python.yml @@ -14,11 +14,10 @@ jobs: strategy: fail-fast: false matrix: - # Using macos-12 bc: - # 1. they removed support for python 3.8 & 3.9 on macos-14 (now macos-latest) - # - See: https://github.com/actions/setup-python/issues/696#issuecomment-2071769156 - # 2. `test_pose` fails on macos-13 and macos-14 (but not macos-12) - os: [ubuntu-20.04, macos-12] + # We use ubuntu-22.04 and macos-13 because they are the final Github runner images + # with Python 3.8 pre-installed. Brain OS images based on Jetpack 5 use 3.8 as the + # system Python, so we want to test 3.8 even though it is EOL. + os: [ubuntu-22.04, macos-13] python-version: ['3.8', '3.9', '3.10', '3.11', '3.12'] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 25f92b1c..ac4a0ca5 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -12,7 +12,7 @@ on: jobs: deploy: name: Deploy to GitHub Pages - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v2 with: diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index af3e150b..f0e801ee 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -14,7 +14,7 @@ concurrency: jobs: pre-commit-check: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v3 - uses: actions/setup-python@v3 diff --git a/.github/workflows/pypi-release.yml b/.github/workflows/pypi-release.yml index 0db671da..1ccec846 100644 --- a/.github/workflows/pypi-release.yml +++ b/.github/workflows/pypi-release.yml @@ -12,7 +12,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-20.04, macos-12] + os: [ubuntu-22.04, macos-13] steps: - uses: actions/checkout@v4 with: @@ -41,7 +41,7 @@ jobs: build_sdist: name: Build source distribution - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 - name: Build sdist @@ -52,7 +52,7 @@ jobs: upload_pypi: needs: [build_wheels, build_sdist] - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 if: github.event_name == 'release' && github.event.action == 'published' steps: - uses: actions/download-artifact@v3 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a67f9c91..1a063eb7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,7 +13,7 @@ jobs: # Based on https://dev.to/eugenebabichenko/automated-multi-platform-releases-with-github-actions-1abg create_release: name: Create Release and Provide Upload URL. - runs-on: ubuntu-20.04 # OS to create release, not for build + runs-on: ubuntu-22.04 # OS to create release, not for build outputs: # This job will provide URL for build jobs to use for uploading assets upload_url: ${{ steps.create_release.outputs.upload_url }} @@ -39,7 +39,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-20.04] + os: [ubuntu-22.04] fail-fast: false steps: @@ -49,7 +49,7 @@ jobs: - name: Install system dependencies (Ubuntu) run: ./scripts/install_deps_ubuntu.sh - if: matrix.os == 'ubuntu-20.04' + if: matrix.os == 'ubuntu-22.04' - name: Install venv dependencies run: ./scripts/build_venv/build_venv.sh diff --git a/.github/workflows/rs.yml b/.github/workflows/rs.yml index 819f1841..1e2996e5 100644 --- a/.github/workflows/rs.yml +++ b/.github/workflows/rs.yml @@ -12,7 +12,7 @@ jobs: strategy: matrix: include: - - os: ubuntu-20.04 + - os: ubuntu-22.04 fail-fast: false steps: - uses: actions/checkout@v3 @@ -24,7 +24,7 @@ jobs: working-directory: ./scripts run: | ./install_deps_ubuntu.sh - if: matrix.os == 'ubuntu-20.04' + if: matrix.os == 'ubuntu-22.04' - name: Download dependencies (proto) run: | From 7833e1540ee6a0dc2d498695650d4dc287170994 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Mon, 16 Dec 2024 14:10:38 -0600 Subject: [PATCH 03/15] Fix python3-venv install for ubuntu-22.04. --- scripts/install_deps_ubuntu.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install_deps_ubuntu.sh b/scripts/install_deps_ubuntu.sh index 401d2523..61ef0bf4 100755 --- a/scripts/install_deps_ubuntu.sh +++ b/scripts/install_deps_ubuntu.sh @@ -36,5 +36,5 @@ sudo apt-get -y install \ libtiff-dev \ ninja-build \ python3-dev \ - python3.8-venv \ + python3-venv \ && sudo apt-get clean From 1986ebbe01dda7e59ac8969d881078568ad691c5 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Mon, 16 Dec 2024 14:21:07 -0600 Subject: [PATCH 04/15] Try removing glog from installed deps. It doesn't appear to be used and conflicts with libc++-dev on ubuntu-22.04: https://bugs.launchpad.net/ubuntu/+source/google-glog/+bug/1991919 --- scripts/install_deps_mac.sh | 1 - scripts/install_deps_ubuntu.sh | 1 - 2 files changed, 2 deletions(-) diff --git a/scripts/install_deps_mac.sh b/scripts/install_deps_mac.sh index abd281c3..ee25584f 100755 --- a/scripts/install_deps_mac.sh +++ b/scripts/install_deps_mac.sh @@ -9,7 +9,6 @@ brew install --verbose \ ccache \ ffmpeg \ glew \ - glog \ libjpeg \ libpng \ libtiff \ diff --git a/scripts/install_deps_ubuntu.sh b/scripts/install_deps_ubuntu.sh index 61ef0bf4..c0578c7b 100755 --- a/scripts/install_deps_ubuntu.sh +++ b/scripts/install_deps_ubuntu.sh @@ -26,7 +26,6 @@ sudo apt-get -y install \ libegl1-mesa-dev \ libglew-dev \ libglm-dev \ - libgoogle-glog-dev \ libgtest-dev \ libopencv-dev \ libssl-dev \ From 5a18cd5e14988235d1857c29182b3b6a77481a7b Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Mon, 16 Dec 2024 15:55:00 -0600 Subject: [PATCH 05/15] Downgrade C++ and Rust CI builds to ubuntu-20.04. These rely on a prebuilt virtualenv for build tooling, created by the "Release" job, which would need additional attention. --- .github/workflows/ci.yml | 8 ++++---- .github/workflows/release.yml | 6 +++--- .github/workflows/rs.yml | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 79861a65..42242687 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,13 +20,13 @@ jobs: strategy: matrix: include: - - os: ubuntu-22.04 + - os: ubuntu-20.04 compiler: gcc asan: off - - os: ubuntu-22.04 + - os: ubuntu-20.04 compiler: clang asan: off - - os: ubuntu-22.04 + - os: ubuntu-20.04 compiler: clang asan: on @@ -41,7 +41,7 @@ jobs: working-directory: ./scripts run: | ./install_deps_ubuntu.sh - if: matrix.os == 'ubuntu-22.04' + if: matrix.os == 'ubuntu-20.04' - name: Download dependencies (proto) run: | diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1a063eb7..a67f9c91 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,7 +13,7 @@ jobs: # Based on https://dev.to/eugenebabichenko/automated-multi-platform-releases-with-github-actions-1abg create_release: name: Create Release and Provide Upload URL. - runs-on: ubuntu-22.04 # OS to create release, not for build + runs-on: ubuntu-20.04 # OS to create release, not for build outputs: # This job will provide URL for build jobs to use for uploading assets upload_url: ${{ steps.create_release.outputs.upload_url }} @@ -39,7 +39,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-22.04] + os: [ubuntu-20.04] fail-fast: false steps: @@ -49,7 +49,7 @@ jobs: - name: Install system dependencies (Ubuntu) run: ./scripts/install_deps_ubuntu.sh - if: matrix.os == 'ubuntu-22.04' + if: matrix.os == 'ubuntu-20.04' - name: Install venv dependencies run: ./scripts/build_venv/build_venv.sh diff --git a/.github/workflows/rs.yml b/.github/workflows/rs.yml index 1e2996e5..819f1841 100644 --- a/.github/workflows/rs.yml +++ b/.github/workflows/rs.yml @@ -12,7 +12,7 @@ jobs: strategy: matrix: include: - - os: ubuntu-22.04 + - os: ubuntu-20.04 fail-fast: false steps: - uses: actions/checkout@v3 @@ -24,7 +24,7 @@ jobs: working-directory: ./scripts run: | ./install_deps_ubuntu.sh - if: matrix.os == 'ubuntu-22.04' + if: matrix.os == 'ubuntu-20.04' - name: Download dependencies (proto) run: | From 750dccf37a19db4490eb4fbaad9d841e19d72d73 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Mon, 16 Dec 2024 16:00:12 -0600 Subject: [PATCH 06/15] Revert "Try removing glog from installed deps." This reverts commit 1986ebbe01dda7e59ac8969d881078568ad691c5. --- scripts/install_deps_mac.sh | 1 + scripts/install_deps_ubuntu.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/scripts/install_deps_mac.sh b/scripts/install_deps_mac.sh index ee25584f..abd281c3 100755 --- a/scripts/install_deps_mac.sh +++ b/scripts/install_deps_mac.sh @@ -9,6 +9,7 @@ brew install --verbose \ ccache \ ffmpeg \ glew \ + glog \ libjpeg \ libpng \ libtiff \ diff --git a/scripts/install_deps_ubuntu.sh b/scripts/install_deps_ubuntu.sh index c0578c7b..61ef0bf4 100755 --- a/scripts/install_deps_ubuntu.sh +++ b/scripts/install_deps_ubuntu.sh @@ -26,6 +26,7 @@ sudo apt-get -y install \ libegl1-mesa-dev \ libglew-dev \ libglm-dev \ + libgoogle-glog-dev \ libgtest-dev \ libopencv-dev \ libssl-dev \ From 850ed030d70b6eb0c62a38b7dd88e627daeb8642 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Tue, 17 Dec 2024 09:55:06 -0600 Subject: [PATCH 07/15] Add more debug output to test_pose. --- py/tests/test_lie.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/py/tests/test_lie.py b/py/tests/test_lie.py index a35095f6..70048dd3 100644 --- a/py/tests/test_lie.py +++ b/py/tests/test_lie.py @@ -142,6 +142,8 @@ def test_pose(): for _i in range(50): world_from_robot_now = world_from_robot.evolve(dt) + print("world_from_robot:", world_from_robot.log()) + print("world_from_robot_now:", world_from_robot_now.log()) # computes the error between frame_b of two respective poses # e.g. robot and robot now From d9e961ffe460a0485471d0978148ebea73479e8c Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Tue, 17 Dec 2024 10:21:01 -0600 Subject: [PATCH 08/15] More debug test output. --- py/tests/test_lie.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/py/tests/test_lie.py b/py/tests/test_lie.py index 70048dd3..a412149f 100644 --- a/py/tests/test_lie.py +++ b/py/tests/test_lie.py @@ -147,7 +147,9 @@ def test_pose(): # computes the error between frame_b of two respective poses # e.g. robot and robot now - err = ng.Pose3F64.error(world_from_robot, world_from_robot_now) * (1 / dt) + err_unscaled = ng.Pose3F64.error(world_from_robot, world_from_robot_now) + print("err_unscaled:", err_unscaled) + err = err_unscaled * (1 / dt) print("err:", err) print("tan:", world_from_robot_now.tangent_of_b_in_a) assert np.allclose(err, world_from_robot_now.tangent_of_b_in_a) From ef19cee459b0e02e4d68a3a2d2f022030d6dead0 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Tue, 17 Dec 2024 14:29:38 -0600 Subject: [PATCH 09/15] Make sophus name lookups explicit in Lie bindings. --- py/pybind/lie_pybind.cpp | 60 +++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/py/pybind/lie_pybind.cpp b/py/pybind/lie_pybind.cpp index 03169eb8..09116249 100644 --- a/py/pybind/lie_pybind.cpp +++ b/py/pybind/lie_pybind.cpp @@ -27,7 +27,6 @@ namespace py = pybind11; using namespace pybind11::literals; // to bring in the `_a` literal -using sophus::Pose3F64; // to guarantee that the array is contiguous, we need to use the buffer protocol using py_array = py::array_t; @@ -125,7 +124,7 @@ void bind_lie(py::module_& m) { auto Pose3F64ToProto = [isometry3F64ToProto, PbVec3F64, PbPose, - PbIsometry3F64Tangent](Pose3F64 const& self) { + PbIsometry3F64Tangent](sophus::Pose3F64 const& self) { Eigen::Vector3d lv = self.tangentOfBInA().head<3>(); Eigen::Vector3d av = self.tangentOfBInA().tail<3>(); return PbPose( @@ -140,7 +139,7 @@ void bind_lie(py::module_& m) { }; auto Pose3F64FromProto = [isometry3F64FromProto](py::object proto) { - Pose3F64::Tangent tangent_of_b_in_a = Pose3F64::Tangent::Zero(); + sophus::Pose3F64::Tangent tangent_of_b_in_a = sophus::Pose3F64::Tangent::Zero(); auto tangent = proto.attr("tangent_of_b_in_a"); if (!tangent.is_none()) { auto linear_vel = tangent.attr("linear_velocity"); @@ -160,7 +159,7 @@ void bind_lie(py::module_& m) { py::cast(angular_vel.attr("z"))); } } - return Pose3F64( + return sophus::Pose3F64( isometry3F64FromProto(proto.attr("a_from_b")), py::cast(proto.attr("frame_a")), py::cast(proto.attr("frame_b")), @@ -264,62 +263,62 @@ void bind_lie(py::module_& m) { return self.translation() = x; }); - py::class_(m, "Pose3F64") + py::class_(m, "Pose3F64") .def( py::init< - Pose3F64::Isometry const&, + sophus::Pose3F64::Isometry const&, std::string const&, std::string const&, - Pose3F64::Tangent const&>(), + sophus::Pose3F64::Tangent const&>(), py::arg("a_from_b"), py::arg("frame_a"), py::arg("frame_b"), - py::arg("tangent_of_b_in_a") = Pose3F64::Tangent::Zero()) + py::arg("tangent_of_b_in_a") = sophus::Pose3F64::Tangent::Zero()) .def_property( "frame_a", - [](Pose3F64 const& self) { return self.frameA(); }, - [](Pose3F64& self, std::string const& frame_a) { + [](sophus::Pose3F64 const& self) { return self.frameA(); }, + [](sophus::Pose3F64& self, std::string const& frame_a) { self.frameA() = frame_a; }) .def_property( "frame_b", - [](Pose3F64 const& self) { return self.frameB(); }, - [](Pose3F64& self, std::string const& frame_b) { + [](sophus::Pose3F64 const& self) { return self.frameB(); }, + [](sophus::Pose3F64& self, std::string const& frame_b) { self.frameB() = frame_b; }) .def_property( "a_from_b", - [](Pose3F64 const& self) { return self.aFromB(); }, - [](Pose3F64& self, Pose3F64::Isometry const& a_from_b) { + [](sophus::Pose3F64 const& self) { return self.aFromB(); }, + [](sophus::Pose3F64& self, sophus::Pose3F64::Isometry const& a_from_b) { self.aFromB() = a_from_b; }) .def_property_readonly( - "b_from_a", [](Pose3F64 const& self) { return self.bFromA(); }) + "b_from_a", [](sophus::Pose3F64 const& self) { return self.bFromA(); }) .def_property( "tangent_of_b_in_a", - [](Pose3F64 const& self) { return self.tangentOfBInA(); }, - [](Pose3F64& self, Pose3F64::Tangent const& tangent_of_b_in_a) { + [](sophus::Pose3F64 const& self) { return self.tangentOfBInA(); }, + [](sophus::Pose3F64& self, sophus::Pose3F64::Tangent const& tangent_of_b_in_a) { self.tangentOfBInA() = tangent_of_b_in_a; }) .def_property( "rotation", - [](Pose3F64 const& self) { return self.rotation(); }, - [](Pose3F64& self, sophus::Rotation3F64 const& x) { + [](sophus::Pose3F64 const& self) { return self.rotation(); }, + [](sophus::Pose3F64& self, sophus::Rotation3F64 const& x) { return self.setRotation(x); }) .def_property( "translation", - [](Pose3F64 const& self) { return self.translation(); }, - [](Pose3F64& self, Eigen::Vector3d const& x) { + [](sophus::Pose3F64 const& self) { return self.translation(); }, + [](sophus::Pose3F64& self, Eigen::Vector3d const& x) { return self.translation() = x; }) - .def("inverse", &Pose3F64::inverse) - .def("log", &Pose3F64::log) - .def("evolve", &Pose3F64::evolve) + .def("inverse", &sophus::Pose3F64::inverse) + .def("log", &sophus::Pose3F64::log) + .def("evolve", &sophus::Pose3F64::evolve) .def( "__mul__", - [](Pose3F64 const& a_from_b, Pose3F64 const& b_from_c) { - farm_ng::Expected a_from_c = a_from_b * b_from_c; + [](sophus::Pose3F64 const& a_from_b, sophus::Pose3F64 const& b_from_c) { + farm_ng::Expected a_from_c = a_from_b * b_from_c; if (a_from_c) { return *a_from_c; } @@ -327,13 +326,16 @@ void bind_lie(py::module_& m) { }) .def( "__mul__", - [](Pose3F64 const& a_from_b, Eigen::Vector3d const& point_in_b) { + [](sophus::Pose3F64 const& a_from_b, Eigen::Vector3d const& point_in_b) { return a_from_b.aFromB() * point_in_b; }) .def_static( "error", - [](Pose3F64 const& lhs_a_from_b, Pose3F64 const& rhs_a_from_b) { - farm_ng::Expected err = + [](sophus::Pose3F64 const& lhs_a_from_b, sophus::Pose3F64 const& rhs_a_from_b) { + // Note: error is a friend function defined inline in Pose3, so it can only + // be found via ADL. That requires unqualified name lookup; sophus::error + // would not be found. + farm_ng::Expected err = error(lhs_a_from_b, rhs_a_from_b); if (err) { return err->array(); From 25fffee1104cbbbba0f973921407db5408db0cd5 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Tue, 17 Dec 2024 14:32:28 -0600 Subject: [PATCH 10/15] Make Pose3::error() static instead of a friend. The ADL-based friend lookup is helpful for operators but confusing and unnecessary for regular functions. --- cpp/sophus/lie/pose3.h | 2 +- py/pybind/lie_pybind.cpp | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/cpp/sophus/lie/pose3.h b/cpp/sophus/lie/pose3.h index c1755836..93ffbfd6 100644 --- a/cpp/sophus/lie/pose3.h +++ b/cpp/sophus/lie/pose3.h @@ -102,7 +102,7 @@ class Pose3 { tangent_in_b_); } - friend Expected error( + static Expected error( Pose3 const& lhs_a_from_b, Pose3 const& rhs_a_from_b) { return (lhs_a_from_b.inverse() * rhs_a_from_b) .and_then([](Expected const& pose) -> Expected { diff --git a/py/pybind/lie_pybind.cpp b/py/pybind/lie_pybind.cpp index 09116249..59f33ad0 100644 --- a/py/pybind/lie_pybind.cpp +++ b/py/pybind/lie_pybind.cpp @@ -332,11 +332,8 @@ void bind_lie(py::module_& m) { .def_static( "error", [](sophus::Pose3F64 const& lhs_a_from_b, sophus::Pose3F64 const& rhs_a_from_b) { - // Note: error is a friend function defined inline in Pose3, so it can only - // be found via ADL. That requires unqualified name lookup; sophus::error - // would not be found. farm_ng::Expected err = - error(lhs_a_from_b, rhs_a_from_b); + sophus::Pose3F64::error(lhs_a_from_b, rhs_a_from_b); if (err) { return err->array(); } From 51bf2a5a0ed4214f79bf75ba38142abded0ef77c Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Tue, 17 Dec 2024 14:40:47 -0600 Subject: [PATCH 11/15] Fix clang-format. --- py/pybind/lie_pybind.cpp | 59 ++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/py/pybind/lie_pybind.cpp b/py/pybind/lie_pybind.cpp index 59f33ad0..8e6b1a87 100644 --- a/py/pybind/lie_pybind.cpp +++ b/py/pybind/lie_pybind.cpp @@ -27,6 +27,7 @@ namespace py = pybind11; using namespace pybind11::literals; // to bring in the `_a` literal +using sophus::Pose3F64; // to guarantee that the array is contiguous, we need to use the buffer protocol using py_array = py::array_t; @@ -124,7 +125,7 @@ void bind_lie(py::module_& m) { auto Pose3F64ToProto = [isometry3F64ToProto, PbVec3F64, PbPose, - PbIsometry3F64Tangent](sophus::Pose3F64 const& self) { + PbIsometry3F64Tangent](Pose3F64 const& self) { Eigen::Vector3d lv = self.tangentOfBInA().head<3>(); Eigen::Vector3d av = self.tangentOfBInA().tail<3>(); return PbPose( @@ -139,7 +140,7 @@ void bind_lie(py::module_& m) { }; auto Pose3F64FromProto = [isometry3F64FromProto](py::object proto) { - sophus::Pose3F64::Tangent tangent_of_b_in_a = sophus::Pose3F64::Tangent::Zero(); + Pose3F64::Tangent tangent_of_b_in_a = Pose3F64::Tangent::Zero(); auto tangent = proto.attr("tangent_of_b_in_a"); if (!tangent.is_none()) { auto linear_vel = tangent.attr("linear_velocity"); @@ -159,7 +160,7 @@ void bind_lie(py::module_& m) { py::cast(angular_vel.attr("z"))); } } - return sophus::Pose3F64( + return Pose3F64( isometry3F64FromProto(proto.attr("a_from_b")), py::cast(proto.attr("frame_a")), py::cast(proto.attr("frame_b")), @@ -263,62 +264,62 @@ void bind_lie(py::module_& m) { return self.translation() = x; }); - py::class_(m, "Pose3F64") + py::class_(m, "Pose3F64") .def( py::init< - sophus::Pose3F64::Isometry const&, + Pose3F64::Isometry const&, std::string const&, std::string const&, - sophus::Pose3F64::Tangent const&>(), + Pose3F64::Tangent const&>(), py::arg("a_from_b"), py::arg("frame_a"), py::arg("frame_b"), - py::arg("tangent_of_b_in_a") = sophus::Pose3F64::Tangent::Zero()) + py::arg("tangent_of_b_in_a") = Pose3F64::Tangent::Zero()) .def_property( "frame_a", - [](sophus::Pose3F64 const& self) { return self.frameA(); }, - [](sophus::Pose3F64& self, std::string const& frame_a) { + [](Pose3F64 const& self) { return self.frameA(); }, + [](Pose3F64& self, std::string const& frame_a) { self.frameA() = frame_a; }) .def_property( "frame_b", - [](sophus::Pose3F64 const& self) { return self.frameB(); }, - [](sophus::Pose3F64& self, std::string const& frame_b) { + [](Pose3F64 const& self) { return self.frameB(); }, + [](Pose3F64& self, std::string const& frame_b) { self.frameB() = frame_b; }) .def_property( "a_from_b", - [](sophus::Pose3F64 const& self) { return self.aFromB(); }, - [](sophus::Pose3F64& self, sophus::Pose3F64::Isometry const& a_from_b) { + [](Pose3F64 const& self) { return self.aFromB(); }, + [](Pose3F64& self, Pose3F64::Isometry const& a_from_b) { self.aFromB() = a_from_b; }) .def_property_readonly( - "b_from_a", [](sophus::Pose3F64 const& self) { return self.bFromA(); }) + "b_from_a", [](Pose3F64 const& self) { return self.bFromA(); }) .def_property( "tangent_of_b_in_a", - [](sophus::Pose3F64 const& self) { return self.tangentOfBInA(); }, - [](sophus::Pose3F64& self, sophus::Pose3F64::Tangent const& tangent_of_b_in_a) { + [](Pose3F64 const& self) { return self.tangentOfBInA(); }, + [](Pose3F64& self, Pose3F64::Tangent const& tangent_of_b_in_a) { self.tangentOfBInA() = tangent_of_b_in_a; }) .def_property( "rotation", - [](sophus::Pose3F64 const& self) { return self.rotation(); }, - [](sophus::Pose3F64& self, sophus::Rotation3F64 const& x) { + [](Pose3F64 const& self) { return self.rotation(); }, + [](Pose3F64& self, sophus::Rotation3F64 const& x) { return self.setRotation(x); }) .def_property( "translation", - [](sophus::Pose3F64 const& self) { return self.translation(); }, - [](sophus::Pose3F64& self, Eigen::Vector3d const& x) { + [](Pose3F64 const& self) { return self.translation(); }, + [](Pose3F64& self, Eigen::Vector3d const& x) { return self.translation() = x; }) - .def("inverse", &sophus::Pose3F64::inverse) - .def("log", &sophus::Pose3F64::log) - .def("evolve", &sophus::Pose3F64::evolve) + .def("inverse", &Pose3F64::inverse) + .def("log", &Pose3F64::log) + .def("evolve", &Pose3F64::evolve) .def( "__mul__", - [](sophus::Pose3F64 const& a_from_b, sophus::Pose3F64 const& b_from_c) { - farm_ng::Expected a_from_c = a_from_b * b_from_c; + [](Pose3F64 const& a_from_b, Pose3F64 const& b_from_c) { + farm_ng::Expected a_from_c = a_from_b * b_from_c; if (a_from_c) { return *a_from_c; } @@ -326,14 +327,14 @@ void bind_lie(py::module_& m) { }) .def( "__mul__", - [](sophus::Pose3F64 const& a_from_b, Eigen::Vector3d const& point_in_b) { + [](Pose3F64 const& a_from_b, Eigen::Vector3d const& point_in_b) { return a_from_b.aFromB() * point_in_b; }) .def_static( "error", - [](sophus::Pose3F64 const& lhs_a_from_b, sophus::Pose3F64 const& rhs_a_from_b) { - farm_ng::Expected err = - sophus::Pose3F64::error(lhs_a_from_b, rhs_a_from_b); + [](Pose3F64 const& lhs_a_from_b, Pose3F64 const& rhs_a_from_b) { + farm_ng::Expected err = + Pose3F64::error(lhs_a_from_b, rhs_a_from_b); if (err) { return err->array(); } From 7efcd0a0961327bbf55b4f012f6625a9f7942283 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Wed, 18 Dec 2024 10:22:15 -0600 Subject: [PATCH 12/15] Disable Python CI/release on MacOS pending SWE-538. --- .github/workflows/ci_python.yml | 4 +++- .github/workflows/pypi-release.yml | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci_python.yml b/.github/workflows/ci_python.yml index 84f6dcd9..721d1fd8 100644 --- a/.github/workflows/ci_python.yml +++ b/.github/workflows/ci_python.yml @@ -17,7 +17,9 @@ jobs: # We use ubuntu-22.04 and macos-13 because they are the final Github runner images # with Python 3.8 pre-installed. Brain OS images based on Jetpack 5 use 3.8 as the # system Python, so we want to test 3.8 even though it is EOL. - os: [ubuntu-22.04, macos-13] + # os: [ubuntu-22.04, macos-13] + # TODO(SWE-538): Fix MacOS CI and release actions on macos-13. + os: [ubuntu-22.04] python-version: ['3.8', '3.9', '3.10', '3.11', '3.12'] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/pypi-release.yml b/.github/workflows/pypi-release.yml index 1ccec846..753e2733 100644 --- a/.github/workflows/pypi-release.yml +++ b/.github/workflows/pypi-release.yml @@ -12,7 +12,9 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-22.04, macos-13] + # TODO(SWE-538): Fix MacOS CI and release actions on macos-13. + # os: [ubuntu-22.04, macos-13] + os: [ubuntu-22.04] steps: - uses: actions/checkout@v4 with: From 91109c995f17b1956be64796eb4c32cd9a012024 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Wed, 18 Dec 2024 16:29:47 -0600 Subject: [PATCH 13/15] Fix ambiguous type inference bug. Resolve error between poses to a concrete Eigen::Vector instead of an expression template representation. --- py/pybind/lie_pybind.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/py/pybind/lie_pybind.cpp b/py/pybind/lie_pybind.cpp index 8e6b1a87..16da86c7 100644 --- a/py/pybind/lie_pybind.cpp +++ b/py/pybind/lie_pybind.cpp @@ -332,7 +332,8 @@ void bind_lie(py::module_& m) { }) .def_static( "error", - [](Pose3F64 const& lhs_a_from_b, Pose3F64 const& rhs_a_from_b) { + [](Pose3F64 const& lhs_a_from_b, + Pose3F64 const& rhs_a_from_b) -> Eigen::Vector { farm_ng::Expected err = Pose3F64::error(lhs_a_from_b, rhs_a_from_b); if (err) { From 93163fe8cf5bcdc9ef8c183a8b25417fe9497a8e Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Wed, 18 Dec 2024 16:33:35 -0600 Subject: [PATCH 14/15] Try to make CI logging from pip less verbose. --- .github/workflows/ci_python.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci_python.yml b/.github/workflows/ci_python.yml index 721d1fd8..848b534a 100644 --- a/.github/workflows/ci_python.yml +++ b/.github/workflows/ci_python.yml @@ -29,7 +29,7 @@ jobs: with: python-version: ${{ matrix.python-version }} - name: Install dependencies - run: pip3 install -vvv -e .[dev] + run: pip3 install -v -e .[dev] - name: Run sequential tests run: ./run_python_tests.sh - name: Run asyncio tests From 37cef09575c986308aa96de25fd323ed2ed09bb1 Mon Sep 17 00:00:00 2001 From: Patrick Mihelich Date: Wed, 18 Dec 2024 16:34:04 -0600 Subject: [PATCH 15/15] Revert "Disable Python CI/release on MacOS pending SWE-538." This reverts commit 7efcd0a0961327bbf55b4f012f6625a9f7942283. --- .github/workflows/ci_python.yml | 4 +--- .github/workflows/pypi-release.yml | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci_python.yml b/.github/workflows/ci_python.yml index 848b534a..959a0584 100644 --- a/.github/workflows/ci_python.yml +++ b/.github/workflows/ci_python.yml @@ -17,9 +17,7 @@ jobs: # We use ubuntu-22.04 and macos-13 because they are the final Github runner images # with Python 3.8 pre-installed. Brain OS images based on Jetpack 5 use 3.8 as the # system Python, so we want to test 3.8 even though it is EOL. - # os: [ubuntu-22.04, macos-13] - # TODO(SWE-538): Fix MacOS CI and release actions on macos-13. - os: [ubuntu-22.04] + os: [ubuntu-22.04, macos-13] python-version: ['3.8', '3.9', '3.10', '3.11', '3.12'] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/pypi-release.yml b/.github/workflows/pypi-release.yml index 753e2733..1ccec846 100644 --- a/.github/workflows/pypi-release.yml +++ b/.github/workflows/pypi-release.yml @@ -12,9 +12,7 @@ jobs: strategy: fail-fast: false matrix: - # TODO(SWE-538): Fix MacOS CI and release actions on macos-13. - # os: [ubuntu-22.04, macos-13] - os: [ubuntu-22.04] + os: [ubuntu-22.04, macos-13] steps: - uses: actions/checkout@v4 with: