From b281f62e24f10dccedc3bafa98d3077e5eeb4888 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Wed, 25 Jan 2023 19:46:09 -0800 Subject: [PATCH 1/4] Added a const_iterator back in --- gtsam/nonlinear/Values.h | 31 +++++++++++++++++++++++++++- gtsam/nonlinear/tests/testValues.cpp | 8 +++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/gtsam/nonlinear/Values.h b/gtsam/nonlinear/Values.h index 161df2cba1..f93323202e 100644 --- a/gtsam/nonlinear/Values.h +++ b/gtsam/nonlinear/Values.h @@ -108,8 +108,11 @@ namespace gtsam { typedef KeyValuePair value_type; + /// @name Constructors + /// @{ + /** Default constructor creates an empty Values class */ - Values() {} + Values() = default; /** Copy constructor duplicates all keys and values */ Values(const Values& other); @@ -127,6 +130,7 @@ namespace gtsam { /** Construct from a Values and an update vector: identical to other.retract(delta) */ Values(const Values& other, const VectorValues& delta); + /// @} /// @name Testable /// @{ @@ -137,6 +141,8 @@ namespace gtsam { bool equals(const Values& other, double tol=1e-9) const; /// @} + /// @name Standard Interface + /// @{ /** Retrieve a variable by key \c j. The type of the value associated with * this key is supplied as a template argument to this function. @@ -177,6 +183,29 @@ namespace gtsam { /** whether the config is empty */ bool empty() const { return values_.empty(); } + /// @} + /// @name Iterator + /// @{ + + struct const_iterator { + using const_iterator_type = typename KeyValueMap::const_iterator; + const_iterator_type it_; + const_iterator(const_iterator_type it) : it_(it) {} + std::pair operator*() const { + return {it_->first, *(it_->second)}; + } + bool operator==(const const_iterator& other) const { return it_ == other.it_; } + bool operator!=(const const_iterator& other) const { return it_ != other.it_; } + const_iterator& operator++() { + ++it_; + return *this; + } + }; + + const_iterator begin() { return const_iterator(values_.begin()); } + const_iterator end() { return const_iterator(values_.end()); } + + /// @} /// @name Manifold Operations /// @{ diff --git a/gtsam/nonlinear/tests/testValues.cpp b/gtsam/nonlinear/tests/testValues.cpp index 644b8c84f1..e554a28b1a 100644 --- a/gtsam/nonlinear/tests/testValues.cpp +++ b/gtsam/nonlinear/tests/testValues.cpp @@ -195,6 +195,14 @@ TEST(Values, basic_functions) values.insert(6, M1); values.insert(8, M2); + size_t count = 0; + for (const auto& [key, value] : values) { + count += 1; + if (key == 2 || key == 4) EXPECT_LONGS_EQUAL(3, value.dim()); + if (key == 6 || key == 8) EXPECT_LONGS_EQUAL(6, value.dim()); + } + EXPECT_LONGS_EQUAL(4, count); + #ifdef GTSAM_ALLOW_DEPRECATED_SINCE_V42 // find EXPECT_LONGS_EQUAL(4, values.find(4)->key); From f2fff1936b590e7cea6ab549fba193a1ac025487 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Wed, 25 Jan 2023 21:17:03 -0800 Subject: [PATCH 2/4] Added some more functions and fixed both flag paths. --- gtsam/nonlinear/Values-inl.h | 8 ++--- gtsam/nonlinear/Values.h | 45 +++++++++++++++------------- gtsam/nonlinear/tests/testValues.cpp | 2 -- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/gtsam/nonlinear/Values-inl.h b/gtsam/nonlinear/Values-inl.h index a354e0139f..04c6440a2b 100644 --- a/gtsam/nonlinear/Values-inl.h +++ b/gtsam/nonlinear/Values-inl.h @@ -163,14 +163,14 @@ namespace gtsam { &ValuesCastHelper::cast)), constBegin_( boost::make_transform_iterator( boost::make_filter_iterator(filter, - ((const Values&) values).begin(), - ((const Values&) values).end()), + values._begin(), + values._end()), &ValuesCastHelper::cast)), constEnd_( boost::make_transform_iterator( boost::make_filter_iterator(filter, - ((const Values&) values).end(), - ((const Values&) values).end()), + values._end(), + values._end()), &ValuesCastHelper::cast)) { } diff --git a/gtsam/nonlinear/Values.h b/gtsam/nonlinear/Values.h index f93323202e..48c9bd1ad9 100644 --- a/gtsam/nonlinear/Values.h +++ b/gtsam/nonlinear/Values.h @@ -187,23 +187,36 @@ namespace gtsam { /// @name Iterator /// @{ - struct const_iterator { + struct deref_iterator { using const_iterator_type = typename KeyValueMap::const_iterator; const_iterator_type it_; - const_iterator(const_iterator_type it) : it_(it) {} - std::pair operator*() const { - return {it_->first, *(it_->second)}; + deref_iterator(const_iterator_type it) : it_(it) {} + ConstKeyValuePair operator*() const { return {it_->first, *(it_->second)}; } + std::unique_ptr operator->() { + return std::make_unique(it_->first, *(it_->second)); } - bool operator==(const const_iterator& other) const { return it_ == other.it_; } - bool operator!=(const const_iterator& other) const { return it_ != other.it_; } - const_iterator& operator++() { + bool operator==(const deref_iterator& other) const { + return it_ == other.it_; + } + bool operator!=(const deref_iterator& other) const { return it_ != other.it_; } + deref_iterator& operator++() { ++it_; return *this; } }; - const_iterator begin() { return const_iterator(values_.begin()); } - const_iterator end() { return const_iterator(values_.end()); } + deref_iterator begin() const { return deref_iterator(values_.begin()); } + deref_iterator end() const { return deref_iterator(values_.end()); } + + /** Find an element by key, returning an iterator, or end() if the key was + * not found. */ + deref_iterator find(Key j) const { return deref_iterator(values_.find(j)); } + + /** Find the element greater than or equal to the specified key. */ + deref_iterator lower_bound(Key j) const { return deref_iterator(values_.lower_bound(j)); } + + /** Find the lowest-ordered element greater than the specified key. */ + deref_iterator upper_bound(Key j) const { return deref_iterator(values_.upper_bound(j)); } /// @} /// @name Manifold Operations @@ -363,8 +376,8 @@ namespace gtsam { static KeyValuePair make_deref_pair(const KeyValueMap::iterator::value_type& key_value) { return KeyValuePair(key_value.first, *key_value.second); } - const_iterator begin() const { return boost::make_transform_iterator(values_.begin(), &make_const_deref_pair); } - const_iterator end() const { return boost::make_transform_iterator(values_.end(), &make_const_deref_pair); } + const_iterator _begin() const { return boost::make_transform_iterator(values_.begin(), &make_const_deref_pair); } + const_iterator _end() const { return boost::make_transform_iterator(values_.end(), &make_const_deref_pair); } iterator begin() { return boost::make_transform_iterator(values_.begin(), &make_deref_pair); } iterator end() { return boost::make_transform_iterator(values_.end(), &make_deref_pair); } const_reverse_iterator rbegin() const { return boost::make_transform_iterator(values_.rbegin(), &make_const_deref_pair); } @@ -376,22 +389,12 @@ namespace gtsam { * not found. */ iterator find(Key j) { return boost::make_transform_iterator(values_.find(j), &make_deref_pair); } - /** Find an element by key, returning an iterator, or end() if the key was - * not found. */ - const_iterator find(Key j) const { return boost::make_transform_iterator(values_.find(j), &make_const_deref_pair); } - /** Find the element greater than or equal to the specified key. */ iterator lower_bound(Key j) { return boost::make_transform_iterator(values_.lower_bound(j), &make_deref_pair); } - /** Find the element greater than or equal to the specified key. */ - const_iterator lower_bound(Key j) const { return boost::make_transform_iterator(values_.lower_bound(j), &make_const_deref_pair); } - /** Find the lowest-ordered element greater than the specified key. */ iterator upper_bound(Key j) { return boost::make_transform_iterator(values_.upper_bound(j), &make_deref_pair); } - /** Find the lowest-ordered element greater than the specified key. */ - const_iterator upper_bound(Key j) const { return boost::make_transform_iterator(values_.upper_bound(j), &make_const_deref_pair); } - /** A filtered view of a Values, returned from Values::filter. */ template class Filtered; diff --git a/gtsam/nonlinear/tests/testValues.cpp b/gtsam/nonlinear/tests/testValues.cpp index e554a28b1a..41bd6aead5 100644 --- a/gtsam/nonlinear/tests/testValues.cpp +++ b/gtsam/nonlinear/tests/testValues.cpp @@ -203,7 +203,6 @@ TEST(Values, basic_functions) } EXPECT_LONGS_EQUAL(4, count); -#ifdef GTSAM_ALLOW_DEPRECATED_SINCE_V42 // find EXPECT_LONGS_EQUAL(4, values.find(4)->key); EXPECT_LONGS_EQUAL(4, values_c.find(4)->key); @@ -219,7 +218,6 @@ TEST(Values, basic_functions) EXPECT_LONGS_EQUAL(6, values_c.upper_bound(4)->key); EXPECT_LONGS_EQUAL(4, values.upper_bound(3)->key); EXPECT_LONGS_EQUAL(4, values_c.upper_bound(3)->key); -#endif } /* ************************************************************************* */ From e2f69e0afe4cd1dd8a1a74193e3fa66f5a89c1d5 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Wed, 25 Jan 2023 21:47:33 -0800 Subject: [PATCH 3/4] Forgot [key, value] only works for c++17 --- gtsam/nonlinear/tests/testValues.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gtsam/nonlinear/tests/testValues.cpp b/gtsam/nonlinear/tests/testValues.cpp index 41bd6aead5..758d9a5a32 100644 --- a/gtsam/nonlinear/tests/testValues.cpp +++ b/gtsam/nonlinear/tests/testValues.cpp @@ -196,10 +196,10 @@ TEST(Values, basic_functions) values.insert(8, M2); size_t count = 0; - for (const auto& [key, value] : values) { + for (const auto& it : values) { count += 1; - if (key == 2 || key == 4) EXPECT_LONGS_EQUAL(3, value.dim()); - if (key == 6 || key == 8) EXPECT_LONGS_EQUAL(6, value.dim()); + if (it.key == 2 || it.key == 4) EXPECT_LONGS_EQUAL(3, it.value.dim()); + if (it.key == 6 || it.key == 8) EXPECT_LONGS_EQUAL(6, it.value.dim()); } EXPECT_LONGS_EQUAL(4, count); From d00971d2c9b43c0dc060551463353e9f2c849f73 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Wed, 25 Jan 2023 22:59:22 -0800 Subject: [PATCH 4/4] Use boost::shared ptr --- gtsam/nonlinear/Values.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gtsam/nonlinear/Values.h b/gtsam/nonlinear/Values.h index 48c9bd1ad9..74f22a27df 100644 --- a/gtsam/nonlinear/Values.h +++ b/gtsam/nonlinear/Values.h @@ -192,8 +192,8 @@ namespace gtsam { const_iterator_type it_; deref_iterator(const_iterator_type it) : it_(it) {} ConstKeyValuePair operator*() const { return {it_->first, *(it_->second)}; } - std::unique_ptr operator->() { - return std::make_unique(it_->first, *(it_->second)); + boost::shared_ptr operator->() { + return boost::make_shared(it_->first, *(it_->second)); } bool operator==(const deref_iterator& other) const { return it_ == other.it_;