ECC-1734: GRIB2: typeOfTimeIncrement=1 removes endStep and stepRange

definitions/grib2/templates/template.4.statistical.def

@@ -51,11 +51,15 @@ statisticalProcessesList list(numberOfTimeRanges)
 }
 
 # See GRIB-488. We only support maximum of 2 time ranges
+# ECC-1734: avgd has the undesirable behaviour of removing the endStep and stepRange, which is incorrect.
+#           We contain this undesirable behaviour to pre-mtg2 data only to keep backwards compatibility
+#           but have post-mtg2 interpreted correctly.
+
 if (numberOfTimeRanges == 1 || numberOfTimeRanges == 2) {
   concept stepTypeInternal {
     "instant" = {typeOfStatisticalProcessing=255;}
     "avg"     = {typeOfStatisticalProcessing=0;}
-    "avgd"    = {typeOfStatisticalProcessing=0;typeOfTimeIncrement=1;}
+    "avgd"    = {MTG2Switch=0;typeOfStatisticalProcessing=0;typeOfTimeIncrement=1;}
     "accum"   = {typeOfStatisticalProcessing=1;}
     "max"     = {typeOfStatisticalProcessing=2;}
     "min"     = {typeOfStatisticalProcessing=3;}

src/eccodes/accessor/G2EndStep.cc

@@ -100,6 +100,7 @@ int G2EndStep::unpack_one_time_range_long_(long* val, size_t* len)
     long time_range_unit;
     long time_range_value, typeOfTimeIncrement;
     int add_time_range = 1; /* whether we add lengthOfTimeRange */
+    long MTG2Switch = 0; /* use the MTG2 switch value to deprecate old hacky logic */
 
     grib_handle* h = get_enclosing_handle();
 
@@ -113,13 +114,15 @@ int G2EndStep::unpack_one_time_range_long_(long* val, size_t* len)
         return err;
     if ((err = grib_get_long_internal(h, typeOfTimeIncrement_, &typeOfTimeIncrement)))
         return err;
+    if ((err = grib_get_long_internal(h, "MTG2Switch", &MTG2Switch)))
+        return err;
 
     err = convert_time_range_long_(h, step_units, time_range_unit, &time_range_value);
     if (err != GRIB_SUCCESS)
         return err;
 
-    if (typeOfTimeIncrement == 1) {
-        /* See GRIB-488 */
+    if (typeOfTimeIncrement == 1 && MTG2Switch == 0) {
+        /* See GRIB-488 & ECC-1734 */
         /* Note: For this case, lengthOfTimeRange is not related to step and should not be used to calculate step */
         add_time_range = 0;
         if (is_special_expver(h)) {
@@ -150,6 +153,7 @@ int G2EndStep::unpack_one_time_range_double_(double* val, size_t* len)
     double time_range_value;
     long typeOfTimeIncrement;
     int add_time_range = 1; /* whether we add lengthOfTimeRange */
+    long MTG2Switch = 0; /* use the MTG2 switch value to deprecate old hacky logic */
 
     grib_handle* h = get_enclosing_handle();
 
@@ -165,12 +169,14 @@ int G2EndStep::unpack_one_time_range_double_(double* val, size_t* len)
         return err;
     if ((err = grib_get_long_internal(h, typeOfTimeIncrement_, &typeOfTimeIncrement)))
         return err;
+    if ((err = grib_get_long_internal(h, "MTG2Switch", &MTG2Switch)))
+        return err;
 
     eccodes::Step start_step{ start_step_value, start_step_unit };
     eccodes::Step time_range{ time_range_value, time_range_unit };
 
-    if (typeOfTimeIncrement == 1) {
-        /* See GRIB-488 */
+    if (typeOfTimeIncrement == 1 && MTG2Switch == 0) {
+        /* See GRIB-488 & ECC-1734 */
         /* Note: For this case, lengthOfTimeRange is not related to step and should not be used to calculate step */
         add_time_range = 0;
         if (is_special_expver(h)) {

tests/CMakeLists.txt

@@ -152,6 +152,7 @@ if( HAVE_BUILD_TOOLS )
         grib_ecc-1654
         grib_ecc-1671
         grib_ecc-1708
+        grib_ecc-1734
         grib_ecc-1766
         grib_ecc-1829
         grib_ecc-2140

tests/grib_ecc-1734.sh

@@ -0,0 +1,42 @@
+#!/bin/sh
+# (C) Copyright 2005- ECMWF.
+#
+# This software is licensed under the terms of the Apache Licence Version 2.0
+# which can be obtained at http://www.apache.org/licenses/LICENSE-2.0.
+# 
+# In applying this licence, ECMWF does not waive the privileges and immunities granted to it by
+# virtue of its status as an intergovernmental organisation nor does it submit to any jurisdiction.
+#
+
+. ./include.ctest.sh
+
+# ---------------------------------------------------------
+# This is the test for JIRA issue ECC-1734
+# GRIB2: typeOfTimeIncrement=1 removes endStep and stepRange
+# ---------------------------------------------------------
+
+label=`basename $0 | sed -e 's/\.sh/_test/'`
+
+tempGrib=temp.$label.grib
+tempFilt=temp.$label.filt
+
+sample_grib2=$ECCODES_SAMPLES_PATH/GRIB2.tmpl
+
+tablesVersionLatest=$( ${tools_dir}/grib_get -p tablesVersionLatest $sample_grib2 )
+
+# Set endStep and typeOfTimeIncrement=1. This should result in endStep and stepRange being set to 0 and 0-6 respectively
+cat >$tempFilt<<EOF
+    set tablesVersion=$tablesVersionLatest;
+    set productDefinitionTemplateNumber=8;
+    set startStep=0;
+    set endStep=6;
+    set typeOfTimeIncrement=1;
+    write;
+EOF
+
+# Check that the values for these keys are computed correctly
+${tools_dir}/grib_filter -o $tempGrib $tempFilt $sample_grib2
+grib_check_key_equals $tempGrib endStep,stepRange "6 0-6"
+
+# Clean up
+rm -f $tempGrib $tempFilt