Skip to content

Add lyrical build environment - #565

Open
glpuga wants to merge 10 commits into
mainfrom
glpuga/add_lyrical
Open

Add lyrical build environment#565
glpuga wants to merge 10 commits into
mainfrom
glpuga/add_lyrical

Conversation

@glpuga

@glpuga glpuga commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Proposed changes

  • Adds lyrical to the container
  • Adds lyrical to CI

Type of change

  • 🐛 Bugfix (change which fixes an issue)
  • 🚀 Feature (change which adds functionality)
  • 📚 Documentation (change which fixes or extends documentation)

💥 Breaking change! Explain why a non-backwards compatible change is necessary or remove this line entirely if not applicable.

Checklist

Put an x in the boxes that apply. This is simply a reminder of what we will require before merging your code.

  • Lint and unit tests (if any) pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All commits have been signed for DCO

Additional comments

Anything worth mentioning to the reviewers.

@glpuga

glpuga commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

Issues so far:

  • nav2 for lyrical has not yet been released
  • Uninitialized variable errors (either real or false positives) triggered during the build of two tests.
  • perf missing

@glpuga
glpuga force-pushed the glpuga/add_lyrical branch from 719913d to a020a02 Compare June 4, 2026 17:36
@glpuga

glpuga commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

Current CI failures seem to be totally unrelated to this change. Locally the current draft of the container builds just fine.

@glpuga
glpuga force-pushed the glpuga/add_lyrical branch 8 times, most recently from aff238d to 2b87f11 Compare June 10, 2026 18:47
@glpuga

glpuga commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

@hidmic @nahueespinosa When building in Lyrical, three four tests (see commented tests in changeset) fail due to a maybe-unitialized warning-turned-error. Looks like a fail positive to me, or maybe a change in Eigen. Thoughts on how to go about it?

--- stderr: beluga                                                                      
In file included from /ws/src/beluga/beluga/test/beluga/algorithm/raycasting/test_bresenham.cpp:23:
In member function ‘bool beluga::Bresenham2i::Line<Vector2, Integer>::iterator::operator==(const beluga::Bresenham2i::Line<Vector2, Integer>::iterator&) const [with Vector2 = Eigen::Matrix<int, 2, 1>; Integer = int]’,
    inlined from ‘bool ranges::operator==(const common_iterator<I1, S1>&, const common_iterator<I2, S2>&) [with I1 = beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator; I2 = beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator; S1 = beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator::sentinel; S2 = beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator::sentinel; bool CPP_true = true; typename std::enable_if<(sentinel_for<S1, I2> && CPP_true), int>::type <anonymous> = 0; typename std::enable_if<(sentinel_for<S2, I1> && CPP_true), int>::type <anonymous> = 0; typename std::enable_if<(equality_comparable_with<I1, I2> && CPP_true), int>::type <anonymous> = 0]’ at /usr/include/range/v3/iterator/common_iterator.hpp:276:63,
    inlined from ‘bool ranges::operator!=(const common_iterator<I1, S1>&, const common_iterator<I2, S2>&) [with I1 = beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator; I2 = beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator; S1 = beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator::sentinel; S2 = beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator::sentinel; bool CPP_true = true; typename std::enable_if<(sentinel_for<S1, I2> && CPP_true), int>::type <anonymous> = 0; typename std::enable_if<(sentinel_for<S2, I1> && CPP_true), int>::type <anonymous> = 0]’ at /usr/include/range/v3/iterator/common_iterator.hpp:284:20,
    inlined from ‘constexpr typename std::iterator_traits<_Iterator>::difference_type std::__distance(_InputIterator, _InputIterator, input_iterator_tag) [with _InputIterator = ranges::common_iterator<beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator, beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator::sentinel>]’ at /usr/include/c++/15/bits/stl_iterator_base_funcs.h:90:22,
    inlined from ‘constexpr typename std::iterator_traits<_Iterator>::difference_type std::distance(_InputIterator, _InputIterator) [with _InputIterator = ranges::common_iterator<beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator, beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator::sentinel>]’ at /usr/include/c++/15/bits/stl_iterator_base_funcs.h:153:29,
    inlined from ‘void std::vector<_Tp, _Alloc>::_M_range_initialize(_ForwardIterator, _ForwardIterator, std::forward_iterator_tag) [with _ForwardIterator = ranges::common_iterator<beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator, beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator::sentinel>; _Tp = Eigen::Matrix<int, 2, 1>; _Alloc = std::allocator<Eigen::Matrix<int, 2, 1> >]’ at /usr/include/c++/15/bits/stl_vector.h:1975:18:
/ws/src/beluga/beluga/include/beluga/algorithm/raycasting/bresenham.hpp:170:16: error: ‘((long int*)&*(const beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1, 0, 2, 1>, int>::iterator*)((char*)&<unnamed> + offsetof(ranges::common_iterator<beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1, 0, 2, 1>, int>::iterator, beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1, 0, 2, 1>, int>::iterator::sentinel>,ranges::common_iterator<beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1, 0, 2, 1>, int>::iterator, beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1, 0, 2, 1>, int>::iterator::sentinel>::data_.ranges::variant<beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1, 0, 2, 1>, int>::iterator, beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1, 0, 2, 1>, int>::iterator::sentinel>::<unnamed>.ranges::detail::variant_data_<meta::list<ranges::detail::indexed_datum<beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1, 0, 2, 1>, int>::iterator, std::integral_constant<long unsigned int, 0> >, ranges::detail::indexed_datum<beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1, 0, 2, 1>, int>::iterator::sentinel, std::integral_constant<long unsigned int, 1> > >, true>::type::<unnamed>)))[1]’ may be used uninitialized [-Werror=maybe-uninitialized]
  170 |         return x_ == other.x_ && y_ == other.y_ && xstep_ == other.xstep_ && ystep_ == other.ystep_ &&
      |                ^~
In file included from /usr/include/c++/15/bits/stl_construct.h:62,
                 from /usr/include/c++/15/bits/stl_tempbuf.h:61,
                 from /usr/include/c++/15/memory:68,
                 from /usr/include/gtest/gtest.h:55,
                 from /ws/src/beluga/beluga/test/beluga/algorithm/raycasting/test_bresenham.cpp:15:
/usr/include/c++/15/bits/stl_iterator_base_funcs.h: In function ‘void std::vector<_Tp, _Alloc>::_M_range_initialize(_ForwardIterator, _ForwardIterator, std::forward_iterator_tag) [with _ForwardIterator = ranges::common_iterator<beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator, beluga::Bresenham2i::Line<Eigen::Matrix<int, 2, 1>, int>::iterator::sentinel>; _Tp = Eigen::Matrix<int, 2, 1>; _Alloc = std::allocator<Eigen::Matrix<int, 2, 1> >]’:
/usr/include/c++/15/bits/stl_iterator_base_funcs.h:153:29: note: ‘<anonymous>’ declared here
  153 |       return std::__distance(__first, __last,
      |              ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
  154 |                              std::__iterator_category(__first));
      |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
gmake[2]: *** [test/beluga/CMakeFiles/test_beluga.dir/build.make:149: test/beluga/CMakeFiles/test_beluga.dir/algorithm/raycasting/test_bresenham.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:1139: test/beluga/CMakeFiles/test_beluga.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2

@glpuga
glpuga force-pushed the glpuga/add_lyrical branch 2 times, most recently from befaf04 to 726ff27 Compare June 10, 2026 18:58
@hidmic

hidmic commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Hmm, it does look like the compiler didn't like one of the many tricks ranges-v3 uses. Nothing in the code suggests a member of the iterator or the iterator itself could be uninitialized. #pragmas to disable the warning for those tests?

@glpuga

glpuga commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Hmm, it does look like the compiler didn't like one of the many tricks ranges-v3 uses. Nothing in the code suggests a member of the iterator or the iterator itself could be uninitialized. #pragmas to disable the warning for those tests?

I thought about that, it makes CI work again, but it's pure chance this hit four unit tests and not the main prod code. Anyone using beluga-the-library might be bitten like this.

@glpuga
glpuga force-pushed the glpuga/add_lyrical branch 2 times, most recently from 2cfb9c4 to 741edce Compare June 12, 2026 19:56
@glpuga glpuga changed the title [WIP] Add lyrical build environment Add lyrical build environment Jun 12, 2026
@glpuga

glpuga commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

As discussed elsewhere, since this is a ranges_v3 issue, most likely a false positive, and since users of the library are not forced to use -Werror, we can just disable these warnings from our tests using pragmas.

@glpuga
glpuga force-pushed the glpuga/add_lyrical branch 2 times, most recently from d7d8fe2 to 45ab002 Compare June 12, 2026 20:04
@glpuga
glpuga marked this pull request as ready for review June 12, 2026 20:05
@glpuga
glpuga force-pushed the glpuga/add_lyrical branch from 45ab002 to b99c5bc Compare June 12, 2026 20:07
@glpuga

glpuga commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

Related to #564

@glpuga
glpuga force-pushed the glpuga/add_lyrical branch from b99c5bc to 777bb62 Compare June 12, 2026 22:42
@glpuga
glpuga force-pushed the glpuga/add_lyrical branch 3 times, most recently from dd6f8a2 to 48f463f Compare June 24, 2026 21:54
@glpuga
glpuga force-pushed the glpuga/add_lyrical branch from 48f463f to 712413d Compare July 3, 2026 16:53
@glpuga
glpuga force-pushed the glpuga/add_lyrical branch 5 times, most recently from 1fb229a to 84897cd Compare July 3, 2026 20:42
glpuga added 10 commits July 6, 2026 15:40
Signed-off-by: Gerardo Puga <glpuga@gmail.com>
Signed-off-by: Gerardo Puga <glpuga@gmail.com>
Signed-off-by: Gerardo Puga <glpuga@gmail.com>
Signed-off-by: Gerardo Puga <glpuga@gmail.com>
Signed-off-by: Gerardo Puga <glpuga@gmail.com>
Signed-off-by: Gerardo Puga <glpuga@gmail.com>
Signed-off-by: Gerardo Puga <glpuga@gmail.com>
Signed-off-by: Gerardo Puga <glpuga@gmail.com>
Signed-off-by: Gerardo Puga <glpuga@gmail.com>
Signed-off-by: Gerardo Puga <glpuga@gmail.com>
@glpuga
glpuga force-pushed the glpuga/add_lyrical branch from 84897cd to d5e9ae9 Compare July 6, 2026 18:44
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