From 4b774f507b7dc11bc186aef2b0f25fc6d0656682 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Mon, 19 Mar 2018 18:03:32 +0300 Subject: [PATCH] Detailed diagnostic information for transit section building. --- generator/transit_generator.cpp | 6 +- transit/transit_graph_data.cpp | 102 ++++++++----------- transit/transit_graph_data.hpp | 5 +- transit/transit_tests/transit_graph_test.cpp | 2 +- transit/transit_tests/transit_test.cpp | 11 +- transit/transit_types.hpp | 25 ++++- 6 files changed, 70 insertions(+), 81 deletions(-) diff --git a/generator/transit_generator.cpp b/generator/transit_generator.cpp index db2a11267f..bf96c3b003 100644 --- a/generator/transit_generator.cpp +++ b/generator/transit_generator.cpp @@ -176,13 +176,13 @@ void ProcessGraph(string const & mwmPath, TCountryId const & countryId, { CalculateBestPedestrianSegments(mwmPath, countryId, data); data.Sort(); - CHECK(data.IsValid(), (mwmPath)); + data.CheckValid(); } void BuildTransit(string const & mwmDir, TCountryId const & countryId, string const & osmIdToFeatureIdsPath, string const & transitDir) { - LOG(LINFO, ("Building transit section for", countryId)); + LOG(LINFO, ("Building transit section for", countryId, "mwmDir:", mwmDir)); Platform::FilesList graphFiles; Platform::GetFilesByExt(my::AddSlashIfNeeded(transitDir), TRANSIT_FILE_EXTENSION, graphFiles); @@ -207,7 +207,7 @@ void BuildTransit(string const & mwmDir, TCountryId const & countryId, return; // Empty transit section. ProcessGraph(mwmPath, countryId, mapping, jointData); - CHECK(jointData.IsValid(), (mwmPath, countryId)); + jointData.CheckValid(); FilesContainerW cont(mwmPath, FileWriter::OP_WRITE_EXISTING); FileWriter writer = cont.GetWriter(TRANSIT_FILE_TAG); diff --git a/transit/transit_graph_data.cpp b/transit/transit_graph_data.cpp index 3d71976354..73e087cec7 100644 --- a/transit/transit_graph_data.cpp +++ b/transit/transit_graph_data.cpp @@ -35,46 +35,31 @@ struct SortVisitor } }; -struct IsValidVisitor +struct CheckValidVisitor { - template - void operator()(Cont const & c, char const * /* name */ ) + template + void operator()(Cont const & c, char const * name) { - m_isValid = m_isValid && ::IsValid(c); + CheckValid(c, name); } - - bool IsValid() const { return m_isValid; } - -private: - bool m_isValid = true; }; -struct IsUniqueVisitor +struct CheckUniqueVisitor { - template - void operator()(Cont const & c, char const * /* name */) + template + void operator()(Cont const & c, char const * name) { - m_isUnique = m_isUnique && (adjacent_find(c.cbegin(), c.cend()) == c.cend()); + CheckUnique(c, name); } - - bool IsUnique() const { return m_isUnique; } - -private: - bool m_isUnique = true; }; -struct IsSortedVisitor +struct CheckSortedVisitor { - template - void operator()(Cont const & c, char const * /* name */) + template + void operator()(Cont const & c, char const * name) { - m_isSorted = m_isSorted && is_sorted(c.cbegin(), c.cend()); + CheckSorted(c, name); } - - bool IsSorted() const { return m_isSorted; } - -private: - bool m_isSorted = true; }; template @@ -136,13 +121,14 @@ void UpdateItems(set const & ids, vector & items) } template -void ReadItems(uint32_t start, uint32_t end, NonOwningReaderSource & src, vector & itmes) +void ReadItems(uint32_t start, uint32_t end, string const & name, NonOwningReaderSource & src, + vector & items) { Deserializer deserializer(src); - CHECK_EQUAL(src.Pos(), start, ("Wrong", TRANSIT_FILE_TAG, "section format.")); - deserializer(itmes); - CHECK_EQUAL(src.Pos(), end, ("Wrong", TRANSIT_FILE_TAG, "section format.")); - CHECK(IsValidSortedUnique(itmes), ()); + CHECK_EQUAL(src.Pos(), start, ("Wrong", TRANSIT_FILE_TAG, "section format. Table name:", name)); + deserializer(items); + CHECK_EQUAL(src.Pos(), end, ("Wrong", TRANSIT_FILE_TAG, "section format. Table name:", name)); + CheckValidSortedUnique(items, name); } } // namespace @@ -339,14 +325,22 @@ void GraphData::Clear() Visit(v); } -bool GraphData::IsValid() const +void GraphData::CheckValid() const { - if (!IsSorted() || !IsUnique()) - return false; + { + CheckSortedVisitor v; + Visit(v); + } - IsValidVisitor v; - Visit(v); - return v.IsValid(); + { + CheckUniqueVisitor v; + Visit(v); + } + + { + CheckValidVisitor v; + Visit(v); + } } bool GraphData::IsEmpty() const @@ -365,7 +359,7 @@ void GraphData::Sort() void GraphData::ClipGraph(vector const & borders) { Sort(); - CHECK(IsValid(), ()); + CheckValid(); ClipLines(borders); ClipStops(); ClipNetworks(); @@ -373,7 +367,7 @@ void GraphData::ClipGraph(vector const & borders) ClipTransfer(); ClipEdges(); ClipShapes(); - CHECK(IsValid(), ()); + CheckValid(); } void GraphData::SetGateBestPedestrianSegment(size_t gateIdx, SingleMwmSegment const & s) @@ -382,20 +376,6 @@ void GraphData::SetGateBestPedestrianSegment(size_t gateIdx, SingleMwmSegment co m_gates[gateIdx].SetBestPedestrianSegment(s); } -bool GraphData::IsUnique() const -{ - IsUniqueVisitor v; - Visit(v); - return v.IsUnique(); -} - -bool GraphData::IsSorted() const -{ - IsSortedVisitor v; - Visit(v); - return v.IsSorted(); -} - void GraphData::ClipLines(vector const & borders) { // Set with stop ids with stops which are inside |borders|. @@ -535,37 +515,37 @@ void GraphData::ReadHeader(NonOwningReaderSource & src) void GraphData::ReadStops(NonOwningReaderSource & src) { - ReadItems(m_header.m_stopsOffset, m_header.m_gatesOffset, src, m_stops); + ReadItems(m_header.m_stopsOffset, m_header.m_gatesOffset, "stops", src, m_stops); } void GraphData::ReadGates(NonOwningReaderSource & src) { - ReadItems(m_header.m_gatesOffset, m_header.m_edgesOffset, src, m_gates); + ReadItems(m_header.m_gatesOffset, m_header.m_edgesOffset, "gates", src, m_gates); } void GraphData::ReadEdges(NonOwningReaderSource & src) { - ReadItems(m_header.m_edgesOffset, m_header.m_transfersOffset, src, m_edges); + ReadItems(m_header.m_edgesOffset, m_header.m_transfersOffset, "edges", src, m_edges); } void GraphData::ReadTransfers(NonOwningReaderSource & src) { - ReadItems(m_header.m_transfersOffset, m_header.m_linesOffset, src, m_transfers); + ReadItems(m_header.m_transfersOffset, m_header.m_linesOffset, "transfers", src, m_transfers); } void GraphData::ReadLines(NonOwningReaderSource & src) { - ReadItems(m_header.m_linesOffset, m_header.m_shapesOffset, src, m_lines); + ReadItems(m_header.m_linesOffset, m_header.m_shapesOffset, "lines", src, m_lines); } void GraphData::ReadShapes(NonOwningReaderSource & src) { - ReadItems(m_header.m_shapesOffset, m_header.m_networksOffset, src, m_shapes); + ReadItems(m_header.m_shapesOffset, m_header.m_networksOffset, "shapes", src, m_shapes); } void GraphData::ReadNetworks(NonOwningReaderSource & src) { - ReadItems(m_header.m_networksOffset, m_header.m_endOffset, src, m_networks); + ReadItems(m_header.m_networksOffset, m_header.m_endOffset, "networks", src, m_networks); } } // namespace transit } // namespace routing diff --git a/transit/transit_graph_data.hpp b/transit/transit_graph_data.hpp index 6f013bf256..ba9734c456 100644 --- a/transit/transit_graph_data.hpp +++ b/transit/transit_graph_data.hpp @@ -134,7 +134,7 @@ public: void DeserializeForCrossMwm(Reader & reader); void AppendTo(GraphData const & rhs); void Clear(); - bool IsValid() const; + void CheckValid() const; bool IsEmpty() const; /// \brief Sorts all class fields by their ids. @@ -160,9 +160,6 @@ private: visitor(m_lines, "lines"), visitor(m_shapes, "shapes"), visitor(m_networks, "networks")) - bool IsUnique() const; - bool IsSorted() const; - /// \brief Clipping |m_lines| with |borders|. /// \details After a call of the method the following stop ids in |m_lines| are left: /// * stops inside |borders| diff --git a/transit/transit_tests/transit_graph_test.cpp b/transit/transit_tests/transit_graph_test.cpp index 5aae5243cd..da2d8e50ec 100644 --- a/transit/transit_tests/transit_graph_test.cpp +++ b/transit/transit_tests/transit_graph_test.cpp @@ -515,7 +515,7 @@ void SerializeAndDeserializeGraph(GraphData & src, GraphData & dst) MemReader reader(buffer.data(), buffer.size()); dst.DeserializeAll(reader); - TEST(dst.IsValid(), ()); + dst.CheckValid(); } // ^ diff --git a/transit/transit_tests/transit_test.cpp b/transit/transit_tests/transit_test.cpp index 01df23c846..54f4b0f14b 100644 --- a/transit/transit_tests/transit_test.cpp +++ b/transit/transit_tests/transit_test.cpp @@ -86,16 +86,13 @@ UNIT_TEST(Transit_HeaderRewriting) namespace { -UNIT_TEST(Transit_IsValidSortedUnique) +UNIT_TEST(Transit_CheckValidSortedUnique) { vector const networks = {Network(1 /* id */, "Title 1"), Network(2 /* id */, "Title 2")}; - TEST(IsValidSortedUnique(networks), ()); + CheckValidSortedUnique(networks, "networks"); - vector const stops(5); - TEST(!IsValidSortedUnique(stops), ()); - - vector const shapeIds = {ShapeId(1, 2), ShapeId(1, 2)}; - TEST(!IsValidSortedUnique(shapeIds), ()); + vector const shapeIds = {ShapeId(1, 2), ShapeId(1, 3)}; + CheckValidSortedUnique(shapeIds, "shapeIds"); } UNIT_TEST(Transit_HeaderSerialization) diff --git a/transit/transit_types.hpp b/transit/transit_types.hpp index 6de7298dff..4918714328 100644 --- a/transit/transit_types.hpp +++ b/transit/transit_types.hpp @@ -5,7 +5,6 @@ #include "geometry/point2d.hpp" #include "base/newtype.hpp" -#include "base/stl_add.hpp" #include "base/visitor.hpp" #include @@ -465,15 +464,31 @@ private: }; template -bool IsValid(std::vector const & items) +void CheckValid(std::vector const & items, std::string const & name) { - return std::all_of(items.cbegin(), items.cend(), [](Item const & item) { return item.IsValid(); }); + for (auto const & i :items) + CHECK(i.IsValid(), (i, "is not valid. Table name:", name)); } template -bool IsValidSortedUnique(std::vector const & items) +void CheckSorted(std::vector const & items, std::string const & name) { - return IsValid(items) && IsSortedAndUnique(items.cbegin(), items.cend()); + CHECK(std::is_sorted(items.cend(), items.cend()), ("Table is not sorted. Table name:", name)); +} + +template +void CheckUnique(std::vector const & items, std::string const & name) +{ + auto const it = std::adjacent_find(items.cbegin(), items.cend()); + CHECK(it == items.cend(), (*it, "is not unique. Table name:", name)); +} + +template +void CheckValidSortedUnique(std::vector const & items, std::string const & name) +{ + CheckValid(items, name); + CheckSorted(items, name); + CheckUnique(items, name); } EdgeFlags GetEdgeFlags(bool transfer, StopId stopId1, StopId stopId2,