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.

Sign up for GitHub

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

Fixes #1984

Merged
merged 11 commits into from
Jan 20, 2025
+61 −49
Merged

Fixes #1984

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0959bfe
replace #ifdef with #if
varunagrawal Jan 20, 2025
a00335f
fully declare struct U
varunagrawal Jan 20, 2025
aa91159
add Ubuntu 24.04 to the CI
varunagrawal Jan 20, 2025
ba355bc
Merge branch 'develop' into fixes
varunagrawal Jan 20, 2025
3d116d7
pass in this to lambda expression explicitly
varunagrawal Jan 20, 2025
7eb5ad6
use reference capture instead of copy capture
varunagrawal Jan 20, 2025
36758a8
add comment and remove unnecessary copy-constructor
varunagrawal Jan 20, 2025
c2ee7ff
bracket clang and GCC
varunagrawal Jan 20, 2025
5ce3327
don't install boost if no_boost flag is set
varunagrawal Jan 20, 2025
cdb761c
update unix.sh to set Boost flags from env variable correctly
varunagrawal Jan 20, 2025
08fc488
check for boost all in one step
varunagrawal Jan 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions .github/workflows/build-linux.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ name: Linux CI

on: [pull_request]

# Every time you make a push to your PR, it cancel immediately the previous checks,
# and start a new one. The other runner will be available more quickly to your PR.
# Every time you make a push to your PR, it cancel immediately the previous checks,
# and start a new one. The other runner will be available more quickly to your PR.
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true
Expand All @@ -25,11 +25,12 @@ jobs:
# Github Actions requires a single row to be added to the build matrix.
# See https://help.github.com/en/articles/workflow-syntax-for-github-actions.
name: [
ubuntu-20.04-gcc-9,
ubuntu-20.04-clang-9,
ubuntu-22.04-gcc-12,
ubuntu-22.04-clang-14,
]
# "Bracket" the versions from GCC [9-14] and Clang [9-16]
ubuntu-20.04-gcc-9,
ubuntu-20.04-clang-9,
ubuntu-24.04-gcc-14,
ubuntu-24.04-clang-16,
]

build_type: [Debug, Release]
build_unstable: [ON]
Expand All @@ -44,15 +45,15 @@ jobs:
compiler: clang
version: "9"

- name: ubuntu-22.04-gcc-12
os: ubuntu-22.04
- name: ubuntu-24.04-gcc-14
os: ubuntu-24.04
compiler: gcc
version: "11"
version: "14"

- name: ubuntu-22.04-clang-14
os: ubuntu-22.04
- name: ubuntu-24.04-clang-16
os: ubuntu-24.04
compiler: clang
version: "14"
version: "16"

steps:
- name: Checkout
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build-special.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ jobs:
fi
- name: Install Boost
if: runner.os == 'Linux'
if: runner.os == 'Linux' && matrix.flag != 'no_boost'
run: |
sudo apt-get -y install libboost-all-dev
Expand Down
3 changes: 2 additions & 1 deletion gtsam/base/std_optional_serialization.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
*/
#ifdef __GNUC__
#if __GNUC__ >= 7 && __cplusplus >= 201703L
namespace boost { namespace serialization { struct U; } }
// Based on https://github.com/borglab/gtsam/issues/1738, we define U as a complete type.
namespace boost { namespace serialization { struct U{}; } }
dellaert marked this conversation as resolved.
Show resolved Hide resolved
namespace std { template<> struct is_trivially_default_constructible<boost::serialization::U> : std::false_type {}; }
namespace std { template<> struct is_trivially_copy_constructible<boost::serialization::U> : std::false_type {}; }
namespace std { template<> struct is_trivially_move_constructible<boost::serialization::U> : std::false_type {}; }
Expand Down
2 changes: 0 additions & 2 deletions gtsam/base/tests/testStdOptionalSerialization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ class TestOptionalStruct {
TestOptionalStruct() = default;
TestOptionalStruct(const int& opt)
: opt(opt) {}
// A copy constructor is needed for serialization
TestOptionalStruct(const TestOptionalStruct& other) = default;
Copy link
Member

Choose a reason for hiding this comment

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

I think I just reviewed a PR that added these?

Copy link
Collaborator Author

@varunagrawal varunagrawal Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This copy constructor was added 2 years ago.

I am currently getting an error because of this

error: definition of implicit copy assignment operator for 'TestOptionalStruct' is deprecated because it has a user-declared copy constructor [-Werror,-Wdeprecated-copy]

Seems to be that using the compiler generated copy-constructors and using the = default constructor no longer gel well together.

bool operator==(const TestOptionalStruct& other) const {
// check the values are equal
return *opt == *other.opt;
Expand Down
26 changes: 16 additions & 10 deletions gtsam/constrained/InequalityPenaltyFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@

namespace gtsam {

/* ********************************************************************************************* */
InequalityPenaltyFunction::UnaryScalarFunc InequalityPenaltyFunction::function() const {
return [=](const double& x, OptionalJacobian<1, 1> H = {}) -> double { return (*this)(x, H); };
/* ************************************************************************* */
InequalityPenaltyFunction::UnaryScalarFunc InequalityPenaltyFunction::function()
const {
return [this](const double& x, OptionalJacobian<1, 1> H = {}) {
return this->operator()(x, H);
};
}

/* ********************************************************************************************* */
/* ************************************************************************* */
dellaert marked this conversation as resolved.
Show resolved Hide resolved
double RampFunction::Ramp(const double x, OptionalJacobian<1, 1> H) {
if (x < 0) {
if (H) {
Expand All @@ -40,8 +43,9 @@ double RampFunction::Ramp(const double x, OptionalJacobian<1, 1> H) {
}
}

/* ********************************************************************************************* */
double SmoothRampPoly2::operator()(const double& x, OptionalJacobian<1, 1> H) const {
/* ************************************************************************* */
double SmoothRampPoly2::operator()(const double& x,
OptionalJacobian<1, 1> H) const {
if (x <= 0) {
if (H) {
H->setZero();
Expand All @@ -60,8 +64,9 @@ double SmoothRampPoly2::operator()(const double& x, OptionalJacobian<1, 1> H) co
}
}

/* ********************************************************************************************* */
double SmoothRampPoly3::operator()(const double& x, OptionalJacobian<1, 1> H) const {
/* ************************************************************************* */
double SmoothRampPoly3::operator()(const double& x,
OptionalJacobian<1, 1> H) const {
if (x <= 0) {
if (H) {
H->setZero();
Expand All @@ -80,8 +85,9 @@ double SmoothRampPoly3::operator()(const double& x, OptionalJacobian<1, 1> H) co
}
}

/* ********************************************************************************************* */
double SoftPlusFunction::operator()(const double& x, OptionalJacobian<1, 1> H) const {
/* ************************************************************************* */
double SoftPlusFunction::operator()(const double& x,
OptionalJacobian<1, 1> H) const {
if (H) {
H->setConstant(1 / (1 + std::exp(-k_ * x)));
}
Expand Down
12 changes: 6 additions & 6 deletions gtsam/geometry/tests/testSO3.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ TEST(SO3, CrossB) {
Matrix aH1;
for (bool nearZero : {true, false}) {
std::function<Vector3(const Vector3&, const Vector3&)> f =
[=](const Vector3& omega, const Vector3& v) {
[nearZero](const Vector3& omega, const Vector3& v) {
return so3::DexpFunctor(omega, nearZero).crossB(v);
};
for (const Vector3& omega : test_cases::omegas(nearZero)) {
Expand All @@ -351,7 +351,7 @@ TEST(SO3, DoubleCrossC) {
Matrix aH1;
for (bool nearZero : {true, false}) {
std::function<Vector3(const Vector3&, const Vector3&)> f =
[=](const Vector3& omega, const Vector3& v) {
[nearZero](const Vector3& omega, const Vector3& v) {
return so3::DexpFunctor(omega, nearZero).doubleCrossC(v);
};
for (const Vector3& omega : test_cases::omegas(nearZero)) {
Expand All @@ -369,7 +369,7 @@ TEST(SO3, ApplyDexp) {
Matrix aH1, aH2;
for (bool nearZero : {true, false}) {
std::function<Vector3(const Vector3&, const Vector3&)> f =
[=](const Vector3& omega, const Vector3& v) {
[nearZero](const Vector3& omega, const Vector3& v) {
return so3::DexpFunctor(omega, nearZero).applyDexp(v);
};
for (const Vector3& omega : test_cases::omegas(nearZero)) {
Expand All @@ -390,7 +390,7 @@ TEST(SO3, ApplyInvDexp) {
Matrix aH1, aH2;
for (bool nearZero : {true, false}) {
std::function<Vector3(const Vector3&, const Vector3&)> f =
[=](const Vector3& omega, const Vector3& v) {
[nearZero](const Vector3& omega, const Vector3& v) {
return so3::DexpFunctor(omega, nearZero).applyInvDexp(v);
};
for (const Vector3& omega : test_cases::omegas(nearZero)) {
Expand All @@ -412,7 +412,7 @@ TEST(SO3, ApplyLeftJacobian) {
Matrix aH1, aH2;
for (bool nearZero : {true, false}) {
std::function<Vector3(const Vector3&, const Vector3&)> f =
[=](const Vector3& omega, const Vector3& v) {
[nearZero](const Vector3& omega, const Vector3& v) {
return so3::DexpFunctor(omega, nearZero).applyLeftJacobian(v);
};
for (const Vector3& omega : test_cases::omegas(nearZero)) {
Expand All @@ -433,7 +433,7 @@ TEST(SO3, ApplyLeftJacobianInverse) {
Matrix aH1, aH2;
for (bool nearZero : {true, false}) {
std::function<Vector3(const Vector3&, const Vector3&)> f =
[=](const Vector3& omega, const Vector3& v) {
[nearZero](const Vector3& omega, const Vector3& v) {
return so3::DexpFunctor(omega, nearZero).applyLeftJacobianInverse(v);
};
for (const Vector3& omega : test_cases::omegas(nearZero)) {
Expand Down
2 changes: 1 addition & 1 deletion gtsam/navigation/tests/testCombinedImuFactor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ TEST(CombinedImuFactor, FirstOrderPreIntegratedMeasurements) {
auto p = testing::Params();
testing::SomeMeasurements measurements;

auto preintegrated = [=](const Vector3& a, const Vector3& w) {
auto preintegrated = [&](const Vector3& a, const Vector3& w) {
PreintegratedImuMeasurements pim(p, Bias(a, w));
testing::integrateMeasurements(measurements, &pim);
return pim.preintegrated();
Expand Down
4 changes: 2 additions & 2 deletions gtsam/navigation/tests/testImuFactor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ TEST(ImuFactor, PartialDerivative_wrt_Bias) {
Vector3 measuredOmega(0.1, 0, 0);
double deltaT = 0.5;

auto evaluateRotation = [=](const Vector3 biasOmega) {
auto evaluateRotation = [&measuredOmega, &deltaT](const Vector3 biasOmega) {
return Rot3::Expmap((measuredOmega - biasOmega) * deltaT);
};

Expand All @@ -424,7 +424,7 @@ TEST(ImuFactor, PartialDerivativeLogmap) {
// Measurements
Vector3 deltaTheta(0, 0, 0);

auto evaluateLogRotation = [=](const Vector3 delta) {
auto evaluateLogRotation = [&thetaHat](const Vector3 delta) {
return Rot3::Logmap(
Rot3::Expmap(thetaHat).compose(Rot3::Expmap(delta)));
};
Expand Down
6 changes: 3 additions & 3 deletions gtsam/navigation/tests/testManifoldPreintegration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,21 @@ TEST(ManifoldPreintegration, BiasCorrectionJacobians) {
testing::SomeMeasurements measurements;

std::function<Rot3(const Vector3&, const Vector3&)> deltaRij =
[=](const Vector3& a, const Vector3& w) {
[&](const Vector3& a, const Vector3& w) {
ManifoldPreintegration pim(testing::Params(), Bias(a, w));
testing::integrateMeasurements(measurements, &pim);
return pim.deltaRij();
};

std::function<Point3(const Vector3&, const Vector3&)> deltaPij =
[=](const Vector3& a, const Vector3& w) {
[&](const Vector3& a, const Vector3& w) {
ManifoldPreintegration pim(testing::Params(), Bias(a, w));
testing::integrateMeasurements(measurements, &pim);
return pim.deltaPij();
};

std::function<Vector3(const Vector3&, const Vector3&)> deltaVij =
[=](const Vector3& a, const Vector3& w) {
[&](const Vector3& a, const Vector3& w) {
ManifoldPreintegration pim(testing::Params(), Bias(a, w));
testing::integrateMeasurements(measurements, &pim);
return pim.deltaVij();
Expand Down
4 changes: 2 additions & 2 deletions gtsam/navigation/tests/testTangentPreintegration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ TEST(ImuFactor, BiasCorrectionJacobians) {
testing::SomeMeasurements measurements;

std::function<Vector9(const Vector3&, const Vector3&)> preintegrated =
[=](const Vector3& a, const Vector3& w) {
[&](const Vector3& a, const Vector3& w) {
TangentPreintegration pim(testing::Params(), Bias(a, w));
testing::integrateMeasurements(measurements, &pim);
return pim.preintegrated();
Expand Down Expand Up @@ -149,7 +149,7 @@ TEST(TangentPreintegration, Compose) {
TEST(TangentPreintegration, MergedBiasDerivatives) {
testing::SomeMeasurements measurements;

auto f = [=](const Vector3& a, const Vector3& w) {
auto f = [&](const Vector3& a, const Vector3& w) {
TangentPreintegration pim02(testing::Params(), Bias(a, w));
testing::integrateMeasurements(measurements, &pim02);
testing::integrateMeasurements(measurements, &pim02);
Expand Down
Loading