Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: mockingbirdnest/Principia
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 5b1b9fbacb52
Choose a base ref
...
head repository: mockingbirdnest/Principia
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: e0f13df18d39
Choose a head ref
  • 6 commits
  • 7 files changed
  • 1 contributor

Commits on Oct 16, 2021

  1. Iteration on an empty segment.

    pleroy committed Oct 16, 2021
    Copy the full SHA
    ac5254a View commit details
  2. Copy the full SHA
    065a992 View commit details
  3. Copy the full SHA
    ccef170 View commit details
  4. Loops.

    pleroy committed Oct 16, 2021
    Copy the full SHA
    e1a7799 View commit details
  5. After egg's review.

    pleroy committed Oct 16, 2021
    Copy the full SHA
    dbf1958 View commit details
  6. Merge pull request #3153 from pleroy/IteratorFixes

    Fix bugs found while writing tests for DiscreteTrajectory2
    pleroy authored Oct 16, 2021
    Copy the full SHA
    e0f13df View commit details
4 changes: 0 additions & 4 deletions physics/discrete_trajectory_iterator.hpp
Original file line number Diff line number Diff line change
@@ -68,10 +68,6 @@ class DiscreteTrajectoryIterator {
DiscreteTrajectorySegmentIterator<Frame> segment_;
OptionalTimelineConstIterator point_;

// The last time that was seen by the iterator. Used to skip over repeated
// times.
std::optional<Instant> previous_time_;

template<typename F>
friend class physics::DiscreteTrajectorySegment;
friend class physics::DiscreteTrajectoryIteratorTest;
53 changes: 27 additions & 26 deletions physics/discrete_trajectory_iterator_body.hpp
Original file line number Diff line number Diff line change
@@ -15,10 +15,11 @@ DiscreteTrajectoryIterator<Frame>&
DiscreteTrajectoryIterator<Frame>::operator++() {
CHECK(!is_at_end(point_));
auto& point = iterator(point_);
for (;;) {
Instant const previous_time = point->first;
do {
if (point == --segment_->timeline_end()) {
++segment_;
if (segment_ == segment_.end()) {
if (segment_.is_end() || segment_->timeline_empty()) {
point_.reset();
break;
} else {
@@ -27,36 +28,32 @@ DiscreteTrajectoryIterator<Frame>::operator++() {
} else {
++point;
}
if (point->first != previous_time_) {
previous_time_ = point->first;
break;
}
}
} while (point->first == previous_time);
return *this;
}

template<typename Frame>
DiscreteTrajectoryIterator<Frame>&
DiscreteTrajectoryIterator<Frame>::operator--() {
if (is_at_end(point_)) {
segment_ = --segment_.end();
point_ = --segment_->timeline_end();
} else {
auto& point = iterator(point_);
for (;;) {
if (point == segment_->timeline_begin()) {
CHECK(segment_ != segment_.begin());
--segment_;
point = --segment_->timeline_end();
} else {
--point;
}
if (point->first != previous_time_) {
previous_time_ = point->first;
break;
}
}
bool const point_is_at_end = is_at_end(point_);
if (point_is_at_end) {
// Move the iterator to the end of the last segment.
segment_ = --segment_.segments().end();
point_ = segment_->timeline_end();
// Now proceed with the decrement.
}
auto& point = iterator(point_);
std::optional<Instant> const previous_time =
point_is_at_end ? std::nullopt : std::make_optional(point->first);
do {
if (point == segment_->timeline_begin()) {
CHECK(!segment_.is_begin());
--segment_;
point = --segment_->timeline_end();
} else {
--point;
}
} while (point->first == previous_time);
return *this;
}

@@ -113,7 +110,11 @@ DiscreteTrajectoryIterator<Frame>::DiscreteTrajectoryIterator(
DiscreteTrajectorySegmentIterator<Frame> const segment,
OptionalTimelineConstIterator const point)
: segment_(segment),
point_(point) {}
point_(point) {
if (segment_.is_end() || segment_->timeline_empty()) {
point_.reset();
}
}

template<typename Frame>
bool DiscreteTrajectoryIterator<Frame>::is_at_end(
59 changes: 52 additions & 7 deletions physics/discrete_trajectory_iterator_test.cpp
Original file line number Diff line number Diff line change
@@ -61,17 +61,20 @@ class DiscreteTrajectoryIteratorTest : public ::testing::Test {
}
}

void Append(Segments::iterator const it,
Instant const& t,
DegreesOfFreedom<World> const& degrees_of_freedom) {
it->Append(t, degrees_of_freedom);
}

DiscreteTrajectoryIterator<World> MakeBegin(
Segments::const_iterator const it) {
return DiscreteTrajectoryIterator<World>(
DiscreteTrajectorySegmentIterator<World>(segments_.get(), it),
it->timeline_begin());
return it->begin();
}

DiscreteTrajectoryIterator<World> MakeEnd(Segments::const_iterator it) {
return DiscreteTrajectoryIterator<World>(
DiscreteTrajectorySegmentIterator<World>(segments_.get(), ++it),
std::nullopt);
DiscreteTrajectoryIterator<World> MakeEnd(
Segments::const_iterator const it) {
return it->end();
}

// Constructs a list of |n| segments which are properly initialized.
@@ -167,5 +170,47 @@ TEST_F(DiscreteTrajectoryIteratorTest, Equality) {
EXPECT_NE(MakeBegin(segments_->begin()), MakeEnd(--segments_->end()));
}

// Empty segments may exist in a transient manner, we must be able to iterate
// over them.
TEST_F(DiscreteTrajectoryIteratorTest, EmptySegment) {
auto segments = MakeSegments(1);
{
int count = 0;
for (auto const& point : segments->front()) {
++count;
}
EXPECT_EQ(0, count);
}
{
int count = 0;
for (auto it = segments->front().rbegin();
it != segments->front().rend();
++it) {
++count;
}
EXPECT_EQ(0, count);
}
}

// Check that repeated points don't cause confusion regarding the end of a
// segment.
TEST_F(DiscreteTrajectoryIteratorTest, SegmentEnd) {
auto segment0 = segments_->begin();
auto segment1 = std::next(segment0);
auto iterator = segment0->begin();
for (int i = 0; i < 5; ++i) {
++iterator;
}
EXPECT_TRUE(iterator != segment1->end());
}

// Checkt that rbegin() works if the next segment is empty.
TEST_F(DiscreteTrajectoryIteratorTest, EmptyLastSegment) {
auto segments = MakeSegments(2);
auto segment = segments->begin();
Append(segment, t0_, unmoving_origin_);
EXPECT_EQ(t0_, segment->rbegin()->first);
}

} // namespace physics
} // namespace principia
9 changes: 5 additions & 4 deletions physics/discrete_trajectory_segment.hpp
Original file line number Diff line number Diff line change
@@ -52,7 +52,7 @@ class DiscreteTrajectorySegment : public Trajectory<Frame> {
explicit DiscreteTrajectorySegment(
DiscreteTrajectorySegmentIterator<Frame> self);

virtual ~DiscreteTrajectorySegment() = default;
~DiscreteTrajectorySegment() = default;

// Moveable.
DiscreteTrajectorySegment(DiscreteTrajectorySegment&&) = default;
@@ -69,7 +69,7 @@ class DiscreteTrajectorySegment : public Trajectory<Frame> {

// TODO(phl): We probably don't want empty segments.
bool empty() const;
virtual std::int64_t size() const;
std::int64_t size() const;

iterator find(Instant const& t) const;

@@ -120,8 +120,9 @@ class DiscreteTrajectorySegment : public Trajectory<Frame> {
Hermite3<Instant, Position<Frame>> GetInterpolation(
typename Timeline::const_iterator upper) const;

virtual typename Timeline::const_iterator timeline_begin() const;
virtual typename Timeline::const_iterator timeline_end() const;
typename Timeline::const_iterator timeline_begin() const;
typename Timeline::const_iterator timeline_end() const;
bool timeline_empty() const;

std::optional<DownsamplingParameters> downsampling_parameters_;

5 changes: 5 additions & 0 deletions physics/discrete_trajectory_segment_body.hpp
Original file line number Diff line number Diff line change
@@ -305,6 +305,11 @@ DiscreteTrajectorySegment<Frame>::timeline_end() const {
return timeline_.cend();
}

template<typename Frame>
bool DiscreteTrajectorySegment<Frame>::timeline_empty() const {
return timeline_.empty();
}

} // namespace internal_discrete_trajectory_segment
} // namespace physics
} // namespace principia
7 changes: 5 additions & 2 deletions physics/discrete_trajectory_segment_iterator.hpp
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@
#include "absl/container/btree_map.h"
#include "base/macros.hpp"
#include "base/not_null.hpp"
#include "physics/discrete_trajectory_segment_range.hpp"
#include "physics/discrete_trajectory_types.hpp"

namespace principia {
@@ -57,8 +58,10 @@ class DiscreteTrajectorySegmentIterator {
DiscreteTrajectorySegmentIterator(not_null<Segments const*> segments,
typename Segments::const_iterator iterator);

DiscreteTrajectorySegmentIterator begin() const;
DiscreteTrajectorySegmentIterator end() const;
bool is_begin() const;
bool is_end() const;
DiscreteTrajectorySegmentRange<DiscreteTrajectorySegmentIterator>
segments() const;

// Not not_null<> to be default-constructible.
Segments const* segments_ = nullptr;
17 changes: 11 additions & 6 deletions physics/discrete_trajectory_segment_iterator_body.hpp
Original file line number Diff line number Diff line change
@@ -62,15 +62,20 @@ DiscreteTrajectorySegmentIterator<Frame>::DiscreteTrajectorySegmentIterator(
iterator_(iterator) {}

template<typename Frame>
DiscreteTrajectorySegmentIterator<Frame>
DiscreteTrajectorySegmentIterator<Frame>::begin() const {
return DiscreteTrajectorySegmentIterator(segments_, segments_->begin());
bool DiscreteTrajectorySegmentIterator<Frame>::is_begin() const {
return iterator_ == segments_->begin();
}

template<typename Frame>
DiscreteTrajectorySegmentIterator<Frame>
DiscreteTrajectorySegmentIterator<Frame>::end() const {
return DiscreteTrajectorySegmentIterator(segments_, segments_->end());
bool DiscreteTrajectorySegmentIterator<Frame>::is_end() const {
return iterator_ == segments_->end();
}

template<typename Frame>
DiscreteTrajectorySegmentRange<DiscreteTrajectorySegmentIterator<Frame>>
DiscreteTrajectorySegmentIterator<Frame>::segments() const {
return {DiscreteTrajectorySegmentIterator(segments_, segments_->begin()),
DiscreteTrajectorySegmentIterator(segments_, segments_->end())};
}

} // namespace internal_discrete_trajectory_segment_iterator