Add functionality to write turn lookups and read turn penalty files #2306
Conversation
lbud
force-pushed
the
1830_turn_penalties
branch
from
c550171 to
bda253e
Compare
|
k @danpat @TheMarex @MoKob and/or @daniel-j-h 🌴 have at it |
| isCompressed | ||
| ? m_node_info_list[m_compressed_edge_container.GetLastEdgeSourceID( | ||
| edge_from_u)] | ||
| : m_node_info_list[node_u]; |
There was a problem hiding this comment.
The behaviour here looks right, but I think the wording could be changed. I think if I were to come back to this in a few months, isCompressed and ContainsCompressedGeometries might confuse me, as technically, all edges are in the CompressedGeometryContainer.
How about isTrivial and isTrivial(), and the some comments in the CompressdGeometryContainer to explain what Trivial means in this context.
lbud
force-pushed
the
1830_turn_penalties
branch
from
7676bf0 to
989297a
Compare
|
Added a few comments for clarity, replaced the removed assertions, renamed the new compressed geometry fns/vars to |
|
@lbud tests on travis seem to be failing. Otherwise 👍 for merging if that is fixed. |
| // node to the first turn would be the same as from end to end of a segment, | ||
| // which is obviously incorrect and not ideal... | ||
| unpacked_path.front().duration_until_turn = | ||
| std::max(unpacked_path.front().duration_until_turn - source_weight, 1); |
There was a problem hiding this comment.
Actually, this explains why the test fails above. If the phantom node snapps to a segment end point the result of this used to be 0. using 0 instead of 1 here should yield the expected behavior.
|
Travis tests were failing because I was testing a debug assertion in cucumber, so all release builds weren't erroring where I expected they should — which exposed the issue that we really needed to catch any negative edge weight regardless of build type, hence 663c28e. This raised some additional questions which I've added in comments here. |
|
@lbud can you rebase again on |
lbud
force-pushed
the
1830_turn_penalties
branch
from
cac91b0 to
97ffd5d
Compare
|
👌 @TheMarex just rebased after renaming |
lbud
force-pushed
the
1830_turn_penalties
branch
from
77cf1b4 to
410661b
Compare
lbud
force-pushed
the
1830_turn_penalties
branch
from
784dbd3 to
b89164c
Compare
|
Just forced pushed after squash/rebase and then I'll merge once travis passes. |
| NodeID CompressedEdgeContainer::GetLastEdgeSourceID(const EdgeID edge_id) const | ||
| { | ||
| const auto &bucket = GetBucketReference(edge_id); | ||
| BOOST_ASSERT(bucket.size() >= 2); | ||
| BOOST_ASSERT(bucket.size() >= 1); | ||
| return bucket[bucket.size() - 2].node_id; | ||
| } |
There was a problem hiding this comment.
bucket.size() >=1 and access to bucket.size()-2 seems like a bad idea?
There was a problem hiding this comment.
Derp. That might have been my ill-advised comment to consider that non-compressed geometries. Yes this should be 2.
Reopened from #2291.