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

Compile in Ubuntu 22.04, 24.04 & C++17 #270

Draft
wants to merge 4 commits into
base: devel
Choose a base branch
from

Conversation

lucasw
Copy link

@lucasw lucasw commented May 7, 2022

Work-in-progress, there are a couple of warnings I didn't eliminate- one requires rviz changes (OgreVector3.h includes), another in eigen:

In file included from /usr/include/eigen3/Eigen/Core:337,
                 from /usr/include/eigen3/Eigen/Dense:1,
                 from /home/lucasw/catkin_ws/src/fuse/fuse_viz/include/fuse_viz/mapped_covariance_visual.h:41,
                 from /home/lucasw/catkin_ws/src/fuse/fuse_viz/src/mapped_covariance_visual.cpp:30:
/usr/include/eigen3/Eigen/src/Core/products/SelfadjointMatrixVector.h: In function ‘static void Eigen::internal::selfadjoint_product_impl<Lhs, LhsMode, false, Rhs, 0, true>::run(Dest&, const Lhs&, const Rhs&, const Scalar&) [with Dest = Eigen::Block<Eigen::Matrix<double, 1, 1, 0, 1, 1>, -1, 1, false>; Lhs = Eigen::Block<Eigen::Matrix<double, 2, 2>, -1, -1, false>; int LhsMode = 17; Rhs = Eigen::CwiseBinaryOp<Eigen::internal::scalar_product_op<double, double>, const Eigen::CwiseNullaryOp<Eigen::internal::scalar_constant_op<double>, const Eigen::Matrix<double, -1, 1, 0, 2, 1> >, const Eigen::Block<Eigen::Block<Eigen::Matrix<double, 2, 2>, 2, 1, true>, -1, 1, false> >]’:
/usr/include/eigen3/Eigen/src/Core/products/SelfadjointMatrixVector.h:229:7: error: ‘result’ may be used uninitialized [-Werror=maybe-uninitialized]
  227 |     internal::selfadjoint_matrix_vector_product<Scalar, Index, (internal::traits<ActualLhsTypeCleaned>::Flags&RowMajorBit) ? RowMajor : ColMajor,
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  228 |                                                 int(LhsUpLo), bool(LhsBlasTraits::NeedToConjugate), bool(RhsBlasTraits::NeedToConjugate)>::run
      |                                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  229 |       (
      |       ^
  230 |         lhs.rows(),                             // size
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  231 |         &lhs.coeffRef(0,0),  lhs.outerStride(), // lhs info
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  232 |         actualRhsPtr,                           // rhs info
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  233 |         actualDestPtr,                          // result info
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  234 |         actualAlpha                             // scale factor
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  235 |       );
      |       ~
/usr/include/eigen3/Eigen/src/Core/products/SelfadjointMatrixVector.h:41:6: note: by argument 3 of type ‘const double*’ to ‘static void Eigen::internal::selfadjoint_matrix_vector_product<Scalar, Index, StorageOrder, UpLo, ConjugateLhs, ConjugateRhs, Version>::run(Index, const Scalar*, Index, const Scalar*, Scalar*, Scalar) [with Scalar = double; Index = long int; int StorageOrder = 0; int UpLo = 1; bool ConjugateLhs = false; bool ConjugateRhs = false; int Version = 0]’ declared here
   41 | void selfadjoint_matrix_vector_product<Scalar,Index,StorageOrder,UpLo,ConjugateLhs,ConjugateRhs,Version>::run(
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors

That looks like it's entirely in Eigen but I haven't looked closely.

Current workaround is to remove the fuse_viz Werror

@lucasw
Copy link
Author

lucasw commented Feb 4, 2023

I'm trying to make this build in Ubuntu 22.10 but getting:

/home/lucasw/base_catkin_ws/src/other/fuse/fuse_graphs/include/fuse_graphs/hash_graph.h:426:21: error: ‘ceres::Problem::Options::local_parameterization_ownership’ is deprecated: Local Parameterizations are deprecated. Use Manifold and manifold_ownership instead. [-Werror=deprecated-declarations]
  426 |   archive & options.local_parameterization_ownership;

22.10 has ceres 2.1.0

@lucasw lucasw changed the title Compile in Ubuntu 22.04 & C++17 Compile in Ubuntu 22.04, 22.10 & C++17 Feb 4, 2023
@svwilliams
Copy link
Contributor

I swear I've written this before, but I'm not seeing it in this PR history. So either this will be a repeat of a message you have seen before, or I never clicked "send". So apologies either way.

Ceres 2.1 has dramatically changed the implementation of what was previously called LocalParameterization and is now called a Manifold. Making fuse support Ceres 2.1 will require a significant time investment. It is something that I intend to support, but likely only in the ROS2 branch. If migrating all of the fuse LocalParameterization code into the new Manifold classes is something you want to tackle, we can create a noetic-22.10-devel branch or similar to hold those updates. But it is not something I can implement or maintain at this time.

@lucasw
Copy link
Author

lucasw commented Jul 4, 2023

How about I peel out the commits that don't break ci, get those into another pr- probably just the hpp and maybe ogre.h includes- that'll reduce the diff noise with breaking changes here, make it easier for me to maintain a 22.04+ branch.

@svwilliams
Copy link
Contributor

Sounds good. I did push a commit a couple of months ago to get fuse compiling on Ubuntu Jammy. That included some of these changes already.

@lucasw lucasw force-pushed the cpp17 branch 2 times, most recently from 4959dbe to 7be1c54 Compare July 4, 2023 14:20
@lucasw
Copy link
Author

lucasw commented Jul 4, 2023

Right, #326 has the d045cc2 and 98dee7f changes, which only leaves these from b3da98f:

-#include <OgreVector3.h>
+#include <Ogre.h>

@lucasw lucasw changed the title Compile in Ubuntu 22.04, 22.10 & C++17 Compile in Ubuntu 22.04, 24.04 & C++17 Apr 17, 2024
@@ -28,7 +28,7 @@ catkin_package(
###########
## Build ##
###########
add_compile_options(-Wall -Werror)
add_compile_options(-Wall) # -Werror)
Copy link
Author

Choose a reason for hiding this comment

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

I haven't tried restoring Werror here since rebasing these commits on latest, maybe it's not needed now?

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

Successfully merging this pull request may close these issues.

2 participants