From b17f40862cf9f65131f4f869a7dee3458f81f21d Mon Sep 17 00:00:00 2001
From: Daniel Patterson <daniel@mapbox.com>
Date: Thu, 31 Aug 2017 14:51:05 -0700
Subject: [PATCH] If traffic CSV has an empty 4th column, preserve the weight
 on an edge while updating the duration.

---
 features/testbot/weight.feature | 33 +++++++++++++++++++++++++++++++++
 include/updater/source.hpp      |  4 ++--
 src/updater/csv_source.cpp      |  4 +++-
 src/updater/updater.cpp         | 29 +++++++++++++++++++++++------
 4 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/features/testbot/weight.feature b/features/testbot/weight.feature
index 686b0dc17..0fe93bf34 100644
--- a/features/testbot/weight.feature
+++ b/features/testbot/weight.feature
@@ -379,3 +379,36 @@ Feature: Weight tests
             | a,d       | abcd,abcd  | 59.9m    | 6.996,0       | 7s,0s    |
             | a,e       | abcd,ce,ce | 60.1m    | 6.005,2.002,0 | 6s,2s,0s |
             | d,e       | abcd,ce,ce | 39.9m    | 1.991,2.002,0 | 2s,2s,0s |
+
+    @traffic @speed
+    Scenario: Updating speeds without affecting weights.
+        Given the profile file "testbot" initialized with
+        """
+        profile.properties.weight_precision = 3
+        """
+
+        And the node map
+            """
+            a-----------b
+             \         /
+                c----d
+            """
+        And the ways
+            | nodes | highway       | maxspeed |
+            | ab    | living_street | 5        |
+            | acdb  | motorway      | 100      |
+
+        # Note the comma on the last column - this indicates 'keep existing weight value'
+        And the speed file
+            """
+            1,2,100,
+            1,3,5,
+            3,4,5,
+            4,2,5,
+            """
+        And the contract extra arguments "--segment-speed-file {speeds_file}"
+        And the customize extra arguments "--segment-speed-file {speeds_file}"
+
+        When I route I should get
+            | waypoints | route      | distance | weights       | times    |
+            | a,b       | acdb,acdb  | 78.3m    | 11.744,0      | 56.4s,0s  |
diff --git a/include/updater/source.hpp b/include/updater/source.hpp
index 462c6b98d..d03cc76bf 100644
--- a/include/updater/source.hpp
+++ b/include/updater/source.hpp
@@ -49,9 +49,9 @@ struct Segment final
 
 struct SpeedSource final
 {
-    SpeedSource() : speed(0), rate(std::numeric_limits<double>::quiet_NaN()) {}
+    SpeedSource() : speed(0), rate() {}
     unsigned speed;
-    double rate;
+    boost::optional<double> rate;
     std::uint8_t source;
 };
 
diff --git a/src/updater/csv_source.cpp b/src/updater/csv_source.cpp
index c00596f56..e089f8797 100644
--- a/src/updater/csv_source.cpp
+++ b/src/updater/csv_source.cpp
@@ -34,7 +34,9 @@ namespace csv
 SegmentLookupTable readSegmentValues(const std::vector<std::string> &paths)
 {
     CSVFilesParser<Segment, SpeedSource> parser(
-        1, qi::ulong_long >> ',' >> qi::ulong_long, qi::uint_ >> -(',' >> qi::double_));
+        1,
+        qi::ulong_long >> ',' >> qi::ulong_long,
+        qi::uint_ >> -(',' >> (qi::double_ | qi::attr(std::numeric_limits<double>::quiet_NaN()))));
 
     // Check consistency of keys in the result lookup table
     auto result = parser(paths);
diff --git a/src/updater/updater.cpp b/src/updater/updater.cpp
index 569cf3098..c278e2627 100644
--- a/src/updater/updater.cpp
+++ b/src/updater/updater.cpp
@@ -162,13 +162,28 @@ updateSegmentData(const UpdaterConfig &config,
     // closure to convert SpeedSource value to weight and count fallbacks to durations
     std::atomic<std::uint32_t> fallbacks_to_duration{0};
     auto convertToWeight = [&profile_properties, &fallbacks_to_duration](
-        const SpeedSource &value, double distance_in_meters) {
-        double rate = value.rate;
-        if (!std::isfinite(rate))
-        { // use speed value in meters per second as the rate
+        const SegmentWeight &existing_weight, const SpeedSource &value, double distance_in_meters) {
+
+        double rate = std::numeric_limits<double>::quiet_NaN();
+
+        // if value.rate is not set, we fall back to duration
+        //    this happens when there is no 4th column in the input CSV
+        // if value.rate is set but NaN, we keep the existing weight
+        //    this happens when there is an empty 4th column in the input CSV
+        // otherwise, we use the value as the new rate
+        if (!value.rate)
+        {
             ++fallbacks_to_duration;
             rate = value.speed / 3.6;
         }
+        else
+        {
+            rate = *value.rate;
+            if (!std::isfinite(rate))
+            {
+                return existing_weight;
+            }
+        }
 
         if (rate <= 0.)
             return INVALID_SEGMENT_WEIGHT;
@@ -231,7 +246,8 @@ updateSegmentData(const UpdaterConfig &config,
                 {
                     auto segment_length = segment_lengths[segment_offset];
                     auto new_duration = convertToDuration(value->speed, segment_length);
-                    auto new_weight = convertToWeight(*value, segment_length);
+                    auto new_weight =
+                        convertToWeight(fwd_weights_range[segment_offset], *value, segment_length);
                     fwd_was_updated = true;
 
                     fwd_weights_range[segment_offset] = new_weight;
@@ -270,7 +286,8 @@ updateSegmentData(const UpdaterConfig &config,
                 {
                     auto segment_length = segment_lengths[segment_offset];
                     auto new_duration = convertToDuration(value->speed, segment_length);
-                    auto new_weight = convertToWeight(*value, segment_length);
+                    auto new_weight =
+                        convertToWeight(rev_weights_range[segment_offset], *value, segment_length);
                     rev_was_updated = true;
 
                     rev_weights_range[segment_offset] = new_weight;