ECC-1620: Force stepUnits

src/grib_accessor_class_g2end_step.cc

@@ -569,6 +569,10 @@ static int pack_long_(grib_accessor* a, const long end_step_value, const long en
     if ((err = grib_get_long_internal(h, "startStepUnit", &start_step_unit)))
         return err;
 
+    long force_step_units;
+    if ((err= grib_get_long_internal(h, "forceStepUnits", &force_step_units)) != GRIB_SUCCESS)
+        return err;
+
     if (Unit{start_step_unit} == Unit{Unit::Value::MISSING}) {
         grib_context_log(h->context, GRIB_LOG_ERROR,
                          "missing start step unit");
@@ -615,7 +619,15 @@ static int pack_long_(grib_accessor* a, const long end_step_value, const long en
 
     const char* forecast_time_value_key = "forecastTime";
     const char* forecast_time_unit_key = "indicatorOfUnitOfTimeRange";
-    auto [forecast_time_opt, time_range_opt] = find_common_units(start_step.optimize_unit(), time_range.optimize_unit());
+    Step forecast_time_opt;
+    Step time_range_opt;
+    if (Unit{force_step_units} == Unit{Unit::Value::MISSING}) {
+        std::tie(forecast_time_opt, time_range_opt) = find_common_units(start_step.optimize_unit(), time_range.optimize_unit());
+    }
+    else {
+        forecast_time_opt = Step{start_step.value<long>(Unit{force_step_units}), Unit{force_step_units}};
+        time_range_opt = Step{time_range.value<long>(Unit{force_step_units}), Unit{force_step_units}};
+    }
 
     if ((err = grib_set_long_internal(grib_handle_of_accessor(a), self->time_range_value, time_range_opt.value<long>())) != GRIB_SUCCESS)
         return err;
@@ -706,8 +718,12 @@ static int pack_string(grib_accessor* a, const char* val, size_t* len)
 {
     grib_handle* h                   = grib_handle_of_accessor(a);
     int ret = 0;
+    long force_step_units;
+    if ((ret = grib_get_long_internal(h, "forceStepUnits", &force_step_units)) != GRIB_SUCCESS)
+        return ret;
+
     try {
-        Step end_step = step_from_string(val);
+        Step end_step = step_from_string(val, Unit{force_step_units});
         end_step.optimize_unit();
 
         if ((ret = grib_set_long_internal(h, "endStepUnit", end_step.unit().value<long>())) != GRIB_SUCCESS)

src/grib_accessor_class_g2step_range.cc

@@ -199,17 +199,30 @@ static int pack_string(grib_accessor* a, const char* val, size_t* len)
     grib_handle* h                   = grib_handle_of_accessor(a);
     int ret = 0;
 
+    long force_step_units;
+    if ((ret = grib_get_long_internal(h, "forceStepUnits", &force_step_units)) != GRIB_SUCCESS)
+        return ret;
+
     try {
-        std::vector<Step> steps = parse_range(val);
+        std::vector<Step> steps = parse_range(val, Unit{force_step_units});
         if (steps.size() == 0) {
             grib_context_log(a->context, GRIB_LOG_ERROR, "Could not parse step range: %s", val);
             return GRIB_INVALID_ARGUMENT;
         }
 
-        Step step_0 = steps[0];
+        Step step_0;
         Step step_1;
-        if (steps.size() > 1) {
+        if (Unit{force_step_units} == Unit{Unit::Value::MISSING}) {
-            std::tie(step_0, step_1) = find_common_units(steps[0].optimize_unit(), steps[1].optimize_unit());
+            if (steps.size() > 1)
+                std::tie(step_0, step_1) = find_common_units(steps[0].optimize_unit(), steps[1].optimize_unit());
+            else
+                step_0 = steps[0].optimize_unit();
+        }
+        else {
+            step_0 = Step{steps[0].value<long>(Unit{force_step_units}), Unit{force_step_units}};
+            if (steps.size() > 1) {
+                step_1 = Step{steps[1].value<long>(Unit{force_step_units}), Unit{force_step_units}};
+            }
         }
 
         if ((ret = grib_set_long_internal(h, "startStepUnit", step_0.unit().value<long>())))

src/grib_accessor_class_optimal_step_units.cc

@@ -157,7 +157,7 @@ static int pack_long(grib_accessor* a, const long* val, size_t* len)
             supported_units_str += Unit{u}.value<std::string>() + ",";
         supported_units_str.pop_back();
 
-        std::string msg = std::string{"Invalid unit: "} + std::to_string(*val) + " (" + e.what() + ")" + ". Available X tunits are: " + supported_units_str;
+        std::string msg = std::string{"Invalid unit: "} + std::to_string(*val) + " (" + e.what() + ")" + ". Available units are: " + supported_units_str;
         grib_context_log(a->context, GRIB_LOG_ERROR, msg.c_str());
         return GRIB_INVALID_ARGUMENT;
     }

src/grib_accessor_class_step_in_units.cc

@@ -198,7 +198,7 @@ static int unpack_double(grib_accessor* a, double * val, size_t* len)
     return GRIB_SUCCESS;
 }
 
-static int pack_long_new_(grib_accessor* a, const long start_step_value, const long start_step_unit)
+static int pack_long_new_(grib_accessor* a, const long start_step_value, const long start_step_unit, const long force_step_units)
 {
     grib_accessor_step_in_units* self = (grib_accessor_step_in_units*)a;
     grib_handle* h = grib_handle_of_accessor(a);
@@ -236,7 +236,10 @@ static int pack_long_new_(grib_accessor* a, const long start_step_value, const l
         return GRIB_SUCCESS;
     }
 
-    forecast_time.optimize_unit();
+    if (Unit{force_step_units} == Unit{Unit::Value::MISSING}) {
+        forecast_time.optimize_unit();
+    }
+
     if ((err = grib_set_long_internal(h, "startStepUnit", forecast_time.unit().value<long>())) != GRIB_SUCCESS)
         return err;
     if ((err = set_step(h, self->forecast_time_value, self->forecast_time_unit, forecast_time)) != GRIB_SUCCESS)
@@ -272,18 +275,23 @@ static int pack_long(grib_accessor* a, const long* val, size_t* len)
         return GRIB_DECODING_ERROR;
     }
 
-    ret = pack_long_new_(a, *val, start_step_unit);
+    ret = pack_long_new_(a, *val, start_step_unit, force_step_units);
 
     return ret;
 }
 
 static int pack_string(grib_accessor* a, const char* val, size_t* len)
 {
+    grib_handle* h = grib_handle_of_accessor(a);
+    //long force_step_units = Unit(Unit::Value::MISSING).value<long>();
     int ret = GRIB_SUCCESS;
-    try {
+    long force_step_units;
-        Step step = step_from_string(val);
+    if ((ret = grib_get_long_internal(h, "forceStepUnits", &force_step_units)) != GRIB_SUCCESS)
+        return ret;
 
-        if ((ret = pack_long_new_(a, step.value<long>(), step.unit().value<long>())) != GRIB_SUCCESS)
+    try {
+        Step step = step_from_string(val, Unit{force_step_units});
+        if ((ret = pack_long_new_(a, step.value<long>(), step.unit().value<long>(), force_step_units)) != GRIB_SUCCESS)
             return ret;
     }
     catch (std::exception& e) {

src/step.cc

@@ -20,34 +20,44 @@
 #include "step_unit.h"
 #include "step.h"
 
-Step step_from_string(std::string step)
+Step step_from_string(const std::string& step, const Unit& force_unit)
 {
     std::regex re("([0-9.]+)([smhDMYC]?)");
     std::smatch match;
     if (std::regex_match(step, match, re)) {
         if (match.size() == 3) {
             std::string value = match[1];
-            std::string unit = match[2];
+            std::string unit_str = match[2];
-            if (unit.size() == 0) {
+            Unit unit;
-                unit = "h";
+            if (unit_str.size() != 0) {
+                if (force_unit == Unit{Unit::Value::MISSING})
+                    unit = Unit{unit_str};
+                else
+                    throw std::runtime_error("Cannot force unit when unit is specified in step string");
             }
-            Step ret{std::stod(value), Unit{unit}};
+            else {
+                if (force_unit == Unit{Unit::Value::MISSING})
+                    unit = Unit{Unit::Value::HOUR};
+                else
+                    unit = force_unit;
+            }
+            Step ret{std::stod(value), unit};
             return ret;
         }
     }
     throw std::runtime_error("Could not parse step: " + step);
 }
 
-std::vector<Step> parse_range(const std::string& range_str)
+std::vector<Step> parse_range(const std::string& range_str, const Unit& force_unit)
 {
     std::vector<Step> steps;
     std::string::size_type pos = 0;
     std::string::size_type prev = 0;
     while ((pos = range_str.find("-", prev)) != std::string::npos) {
-        steps.push_back(step_from_string(range_str.substr(prev, pos - prev)));
+        steps.push_back(step_from_string(range_str.substr(prev, pos - prev), force_unit));
         prev = pos + 1;
     }
-    steps.push_back(step_from_string(range_str.substr(prev)));
+    steps.push_back(step_from_string(range_str.substr(prev), force_unit));
     return steps;
 }
 
@@ -132,13 +142,14 @@ void Step::init_long(long value, const Unit& unit)
 {
     internal_value_ = value;
     internal_unit_ = unit;
-    unit_ = internal_unit_;
+    unit_ = unit;
 }
 
 void Step::init_double(double value, const Unit& unit)
 {
     auto seconds = Unit::get_converter().unit_to_duration(unit.value<Unit::Value>());
-    init_long(static_cast<long>(value * seconds), Unit{Unit::Value::SECOND});
+    internal_value_ = value * seconds;
+    internal_unit_ = Unit{Unit::Value::SECOND};
     unit_ = unit;
 }
 

src/step.h

@@ -94,8 +94,8 @@ private:
 };
 
 
-Step step_from_string(std::string step);
+Step step_from_string(const std::string& step, const Unit& force_unit);
-std::vector<Step> parse_range(const std::string& range_str);
+std::vector<Step> parse_range(const std::string& range_str, const Unit& force_unit);
 std::pair<Step, Step> find_common_units(const Step& startStep, const Step& endStep);
 
 

tests/grib_ecc-1620.sh

@@ -45,9 +45,49 @@ temp=temp.$label
 temp2=temp_2.$label
 samples_dir=$ECCODES_SAMPLES_PATH
 
+instantaneous_field=$data_dir/reduced_gaussian_surface.grib2
+accumulated_field=$data_dir/reduced_gaussian_sub_area.grib2
+
+# if stepUnits is set, then set the low level keys to stepUnits
+# if stepUnits is not set, then optimise low level keys
+# instant fields:
+low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s,lengthOfTimeRange,indicatorOfUnitForTimeRange:s"
+fn="$instantaneous_field"
+low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s"
+keys__="step,stepUnits:s"
+keys_s="step:s"
+keys_i="step:i,stepUnits:s"
+keys_d="step:d,stepUnits:s"
+
+${tools_dir}/grib_set -s stepUnits=m,step=60 $fn $temp
+grib_check_key_equals $temp "-p $low_level_keys" "60 m"
+grib_check_key_equals $temp "-p $keys_s" "1"
+grib_check_key_equals $temp "-p $keys_s -s stepUnits=m" "60m"
+${tools_dir}/grib_set -s step=60m $fn $temp
+grib_check_key_equals $temp "-p $low_level_keys" "1 h"
+grib_check_key_equals $temp "-p $keys_s" "1"
+grib_check_key_equals $temp "-p $keys_s -s stepUnits=m" "60m"
+
+
+# accumulated fields:
+fn="$accumulated_field"
+low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s,lengthOfTimeRange,indicatorOfUnitForTimeRange:s"
+keys__="step,startStep,endStep,stepRange,stepUnits:s"
+keys_s="step:s,startStep:s,endStep:s,stepRange:s,stepUnits:s"
+keys_i="step:i,startStep:i,endStep:i,stepRange:i,stepUnits:s"
+keys_d="step:d,startStep:d,endStep:d,stepRange:d,stepUnits:s"
+
+${tools_dir}/grib_set -s stepUnits=m,stepRange=60-120 $fn $temp
+grib_check_key_equals $temp "-p $low_level_keys" "60 m 60 m"
+grib_check_key_equals $temp "-p $keys_s" "2 1 2 1-2 h"
+grib_check_key_equals $temp "-p $keys_s -s stepUnits=m" "120m 60m 120m 60m-120m m"
+${tools_dir}/grib_set -s stepRange=60m-120m $fn $temp
+grib_check_key_equals $temp "-p $low_level_keys" "1 h 1 h"
+grib_check_key_equals $temp "-p $keys_s" "2 1 2 1-2 h"
+grib_check_key_equals $temp "-p $keys_s -s stepUnits=m" "120m 60m 120m 60m-120m m"
 
 #### CHECK units
-fn="${data_dir}/reduced_gaussian_sub_area.grib2"
+fn="$accumulated_field"
 low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s,lengthOfTimeRange,indicatorOfUnitForTimeRange:s"
 ${tools_dir}/grib_set -s forecastTime=0,indicatorOfUnitOfTimeRange=h,lengthOfTimeRange=96,indicatorOfUnitForTimeRange=h $fn $temp
 grib_check_key_equals $temp "-p $low_level_keys" "0 h 96 h"
@@ -71,25 +111,21 @@ grib_check_key_equals $temp " -w count=1 -s stepUnits=12h -p step,stepUnits:s" "
 grib_check_key_equals $temp " -w count=1 -s stepUnits=D -p step,stepUnits:s" "4D D"
 
 ${tools_dir}/grib_set -s stepUnits=s,startStep=0,endStep=345600  $fn $temp
-grib_check_key_equals $temp "-p $low_level_keys" "0 h 96 h"
+grib_check_key_equals $temp "-p $low_level_keys" "0 s 345600 s"
 ${tools_dir}/grib_set -s stepUnits=m,startStep=0,endStep=5760  $fn $temp
-grib_check_key_equals $temp "-p $low_level_keys" "0 h 96 h"
+grib_check_key_equals $temp "-p $low_level_keys" "0 m 5760 m"
-${tools_dir}/grib_set -s stepUnits=15m,startStep=0,endStep=384  $fn $temp
-grib_check_key_equals $temp "-p $low_level_keys" "0 h 96 h"
-${tools_dir}/grib_set -s stepUnits=30m,startStep=0,endStep=192  $fn $temp
-grib_check_key_equals $temp "-p $low_level_keys" "0 h 96 h"
 ${tools_dir}/grib_set -s stepUnits=h,startStep=0,endStep=96  $fn $temp
 grib_check_key_equals $temp "-p $low_level_keys" "0 h 96 h"
 ${tools_dir}/grib_set -s stepUnits=6h,startStep=0,endStep=16  $fn $temp
-grib_check_key_equals $temp "-p $low_level_keys" "0 h 96 h"
+grib_check_key_equals $temp "-p $low_level_keys" "0 6h 16 6h"
 ${tools_dir}/grib_set -s stepUnits=12h,startStep=0,endStep=8  $fn $temp
-grib_check_key_equals $temp "-p $low_level_keys" "0 h 96 h"
+grib_check_key_equals $temp "-p $low_level_keys" "0 12h 8 12h"
 ${tools_dir}/grib_set -s stepUnits=D,startStep=0,endStep=4  $fn $temp
-grib_check_key_equals $temp "-p $low_level_keys" "0 h 96 h"
+grib_check_key_equals $temp "-p $low_level_keys" "0 D 4 D"
 
 
 #### CHECK negative forecastTime
-fn="${data_dir}/reduced_gaussian_sub_area.grib2"
+fn="$accumulated_field"
 low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s,lengthOfTimeRange,indicatorOfUnitForTimeRange:s"
 ${tools_dir}/grib_set -s forecastTime=-6,indicatorOfUnitOfTimeRange=h,lengthOfTimeRange=6,indicatorOfUnitForTimeRange=h $fn $temp
 grib_check_key_equals $temp "-p $low_level_keys" "-6 h 6 h"
@@ -103,33 +139,36 @@ grib_check_key_equals $temp "-p stepRange" "-48"
 
 
 #### CHECK: check optimal units are set correctly in GRIB files
-fn="${data_dir}/reduced_gaussian_sub_area.grib2"
+fn="$accumulated_field"
 low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s,lengthOfTimeRange,indicatorOfUnitForTimeRange:s"
 ${tools_dir}/grib_set -s forecastTime=24,indicatorOfUnitOfTimeRange=h,lengthOfTimeRange=1,indicatorOfUnitForTimeRange=D $fn $temp
 grib_check_key_equals $temp    "-p $low_level_keys" "24 h 1 D"
 
 ### TODO(maee): @Shahram: how to make parameters position independent
 ${tools_dir}/grib_set -s stepUnits:s=s,startStep:i=60,endStep:i=180 $temp $temp2
-grib_check_key_equals $temp2   "-p $low_level_keys" "1 m 2 m"
+grib_check_key_equals $temp2   "-p $low_level_keys" "60 s 120 s"
 #${tools_dir}/grib_set -s startStep:i=60,endStep:i=180,stepUnits:s=s $temp $temp2
 #grib_check_key_equals $temp2   "-p $low_level_keys" "1 m 2 m"
 
+# Seconds
 ${tools_dir}/grib_set -s stepUnits:i=13,startStep:i=60,endStep:i=180 $temp $temp2
-grib_check_key_equals $temp2   "-p $low_level_keys" "1 m 2 m"
+grib_check_key_equals $temp2   "-p $low_level_keys" "60 s 120 s"
 ${tools_dir}/grib_set -s stepUnits:s=s,startStep:i=60,endStep:i=180 $temp $temp2
-grib_check_key_equals $temp2   "-p $low_level_keys" "1 m 2 m"
+grib_check_key_equals $temp2   "-p $low_level_keys" "60 s 120 s"
 
+# Minutes
 ${tools_dir}/grib_set -s stepUnits:i=0,startStep:i=60,endStep:i=180 $temp $temp2
-grib_check_key_equals $temp2   "-p $low_level_keys" "1 h 2 h"
+grib_check_key_equals $temp2   "-p $low_level_keys" "60 m 120 m"
 ${tools_dir}/grib_set -s stepUnits:s=m,startStep:i=60,endStep:i=180 $temp $temp2
-grib_check_key_equals $temp2   "-p $low_level_keys" "1 h 2 h"
+grib_check_key_equals $temp2   "-p $low_level_keys" "60 m 120 m"
 
+# Hours
 ${tools_dir}/grib_set -s stepUnits:i=1,startStep:i=60,endStep:i=180 $temp $temp2
 grib_check_key_equals $temp2   "-p $low_level_keys" "60 h 120 h"
 ${tools_dir}/grib_set -s stepUnits:s=h,startStep:i=60,endStep:i=180 $temp $temp2
 grib_check_key_equals $temp2   "-p $low_level_keys" "60 h 120 h"
 
-#fn="${data_dir}/reduced_gaussian_sub_area.grib2"
+#fn="$accumulated_field"
 #low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s,lengthOfTimeRange,indicatorOfUnitForTimeRange:s"
 ##high_level_keys="startStep:s,endStep:s"
 #high_level_keys="startStep:i,endStep:i"
@@ -149,7 +188,7 @@ grib_check_key_equals $temp2   "-p $low_level_keys" "60 h 120 h"
 #exit
 
 #### CHECK: grib_set - endStep + stepUnits
-fn="${data_dir}/reduced_gaussian_sub_area.grib2"
+fn="$accumulated_field"
 low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s,lengthOfTimeRange,indicatorOfUnitForTimeRange:s"
 ${tools_dir}/grib_set -s forecastTime=24,indicatorOfUnitOfTimeRange=h,lengthOfTimeRange=1,indicatorOfUnitForTimeRange=D $fn $temp
 grib_check_key_equals $temp    "-p $low_level_keys" "24 h 1 D"
@@ -211,7 +250,7 @@ ${tools_dir}/grib_set -s stepRange:s=62D-122D $temp $temp2
 grib_check_key_equals $temp2   "-p $low_level_keys" "1488 h 1440 h"
 grib_check_key_equals $temp2   "-p stepRange:s"     "1488-2928"
 
-fn="${data_dir}/reduced_gaussian_surface.grib2"
+fn="$instantaneous_field"
 low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s"
 keys__="step,stepUnits:s"
 keys_s="step:s"
@@ -221,16 +260,18 @@ keys_d="step:d,stepUnits:s"
 ${tools_dir}/grib_set -s forecastTime=59,indicatorOfUnitOfTimeRange=m $fn $temp
 grib_check_key_equals $temp "-p $keys__ -s stepUnits=s" "3540s s"
 grib_check_key_equals $temp "-p $keys__ -s stepUnits=m" "59m m"
-#grib_check_key_equals $temp "-p $keys__ -s stepUnits=h" "0" # TODO(maee): check behaviour (should be 0.983333)
+#grib_check_key_equals $temp "-p $keys__ -s stepUnits=h" "0" # not supported
 grib_check_key_equals $temp "-p $keys_s -s stepUnits=s" "3540s"
 grib_check_key_equals $temp "-p $keys_s -s stepUnits=m" "59m"
-#grib_check_key_equals $temp "-p $keys_s -F"%.2f" -s stepUnits=h" "0.983333" # TODO(maee): check behaviour // See tools for default output format
+#grib_check_key_equals $temp "-p $keys_s -F"%.2f" -s stepUnits=h" "0.98" # not supported
 grib_check_key_equals $temp "-p $keys_i -s stepUnits=s" "3540 s"
 grib_check_key_equals $temp "-p $keys_i -s stepUnits=m" "59 m"
-#grib_check_key_equals $temp "-p $keys_i -s stepUnits=h" "0" # TODO(maee): check behaviour
+#grib_check_key_equals $temp "-p $keys_i -s stepUnits=h" "0" # not supported
 grib_check_key_equals $temp "-p $keys_d -s stepUnits=s" "3540 s"
 grib_check_key_equals $temp "-p $keys_d -s stepUnits=m" "59 m"
-#grib_check_key_equals $temp "-p $keys_d -s stepUnits=h" "0.983333" # TODO(maee): check behaviour
+#grib_check_key_equals $temp "-p $keys_d -s stepUnits=h" "0.983333" # not supported
+
+
 
 
 ${tools_dir}/grib_set -s forecastTime=0,indicatorOfUnitOfTimeRange=m $fn $temp
@@ -242,7 +283,8 @@ grib_check_key_equals $temp "-p $keys_d -s stepUnits=m" "0 m"
 grib_check_key_equals $temp "-p $keys_d -s stepUnits=h" "0 h"
 
 
-fn="${data_dir}/reduced_gaussian_surface.grib2"
+
+fn="$instantaneous_field"
 low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s"
 keys__="step,stepUnits:s"
 keys_s="step:s,stepUnits:s"
@@ -308,13 +350,13 @@ grib_check_key_equals $temp "-p $keys_i" "24 h"
 grib_check_key_equals $temp "-p $keys_d" "24 h"
 
 
-fn="${data_dir}/reduced_gaussian_sub_area.grib2"
+fn="$accumulated_field"
 low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s,lengthOfTimeRange,indicatorOfUnitForTimeRange:s"
 ${tools_dir}/grib_set -s stepRange=60m-2h $fn $temp
 grib_check_key_equals $temp "-p $low_level_keys" "1 h 1 h"
 
 
-fn="${data_dir}/reduced_gaussian_sub_area.grib2"
+fn="$accumulated_field"
 low_level_keys="forecastTime,indicatorOfUnitOfTimeRange:s,lengthOfTimeRange,indicatorOfUnitForTimeRange:s"
 keys__="stepRange,startStep,endStep"
 keys_s="stepRange:s,startStep:s,endStep:s"