From 279eaf6f5101c791ab699c3380f15d268d355194 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Tue, 29 Sep 2020 14:36:43 +0100 Subject: [PATCH] ECC-1150: keys 'lowerLimit' and 'upperLimit' cannot be MISSING --- definitions/grib2/template.4.probability.def | 19 ++++----- ...sor_class_from_scale_factor_scaled_value.c | 22 ++++++---- src/grib_accessor_class_gen.c | 5 ++- tests/CMakeLists.txt | 4 +- tests/grib_ecc-1150.sh | 41 +++++++++++++++++++ tests/grib_ecc-966.sh | 28 +++++++++++++ 6 files changed, 99 insertions(+), 20 deletions(-) create mode 100755 tests/grib_ecc-1150.sh create mode 100755 tests/grib_ecc-966.sh diff --git a/definitions/grib2/template.4.probability.def b/definitions/grib2/template.4.probability.def index ad1af9ff8..deb033b14 100755 --- a/definitions/grib2/template.4.probability.def +++ b/definitions/grib2/template.4.probability.def @@ -1,31 +1,30 @@ # (C) Copyright 2005- ECMWF. -# Forecast probability number +# Forecast probability number unsigned[1] forecastProbabilityNumber : dump; -# Total number of forecast probabilities +# Total number of forecast probabilities unsigned[1] totalNumberOfForecastProbabilities : dump; -# Probability type +# Probability type codetable[1] probabilityType ('4.9.table',masterDir,localDir) : dump; meta probabilityTypeName codetable_title(probabilityType): read_only; - -# Scale factor of lower limit +# Scale factor of lower limit signed[1] scaleFactorOfLowerLimit : can_be_missing,dump ; -# Scaled value of lower limit +# Scaled value of lower limit signed[4] scaledValueOfLowerLimit : can_be_missing,dump ; meta lowerLimit from_scale_factor_scaled_value( - scaleFactorOfLowerLimit, scaledValueOfLowerLimit); + scaleFactorOfLowerLimit, scaledValueOfLowerLimit): can_be_missing; -# Scale factor of upper limit +# Scale factor of upper limit signed[1] scaleFactorOfUpperLimit : can_be_missing,dump; -# Scaled value of upper limit +# Scaled value of upper limit signed[4] scaledValueOfUpperLimit : can_be_missing,dump; meta upperLimit from_scale_factor_scaled_value( - scaleFactorOfUpperLimit, scaledValueOfUpperLimit); + scaleFactorOfUpperLimit, scaledValueOfUpperLimit): can_be_missing; diff --git a/src/grib_accessor_class_from_scale_factor_scaled_value.c b/src/grib_accessor_class_from_scale_factor_scaled_value.c index 04710060e..fe17caabb 100644 --- a/src/grib_accessor_class_from_scale_factor_scaled_value.c +++ b/src/grib_accessor_class_from_scale_factor_scaled_value.c @@ -246,16 +246,22 @@ static int unpack_double(grib_accessor* a, double* val, size_t* len) if ((ret = grib_get_long_internal(hand, self->scaleFactor, &scaleFactor)) != GRIB_SUCCESS) return ret; - /* ECC-966: If scale factor is missing, print error and treat it as zero (as a fallback) */ - if (grib_is_missing(hand, self->scaleFactor, &ret) && ret == GRIB_SUCCESS) { - grib_context_log(a->context, GRIB_LOG_ERROR, - "unpack_double for %s: %s is missing! Setting it to zero", a->name, self->scaleFactor); - scaleFactor = 0; - } - if ((ret = grib_get_long_internal(hand, self->scaledValue, &scaledValue)) != GRIB_SUCCESS) return ret; + if (grib_is_missing(hand, self->scaledValue, &ret) && ret == GRIB_SUCCESS) { + *val = GRIB_MISSING_DOUBLE; + *len = 1; + return GRIB_SUCCESS; + } else { + /* ECC-966: If scale factor is missing, print error and treat it as zero (as a fallback) */ + if (grib_is_missing(hand, self->scaleFactor, &ret) && ret == GRIB_SUCCESS) { + grib_context_log(a->context, GRIB_LOG_ERROR, + "unpack_double for %s: %s is missing! Using zero instead", a->name, self->scaleFactor); + scaleFactor = 0; + } + } + *val = scaledValue; /* The formula is: @@ -289,5 +295,5 @@ static int is_missing(grib_accessor* a) if ((ret = grib_get_long_internal(grib_handle_of_accessor(a), self->scaledValue, &scaledValue)) != GRIB_SUCCESS) return ret; - return ((scaleFactor == GRIB_MISSING_LONG) | (scaledValue == GRIB_MISSING_LONG)); + return ((scaleFactor == GRIB_MISSING_LONG) || (scaledValue == GRIB_MISSING_LONG)); } diff --git a/src/grib_accessor_class_gen.c b/src/grib_accessor_class_gen.c index 797cf5145..3b4ca037a 100644 --- a/src/grib_accessor_class_gen.c +++ b/src/grib_accessor_class_gen.c @@ -275,7 +275,10 @@ static int unpack_long(grib_accessor* a, long* v, size_t* len) double val = 0.0; size_t l = 1; grib_unpack_double(a, &val, &l); - *v = (long)val; + if (val == GRIB_MISSING_DOUBLE) + *v = GRIB_MISSING_LONG; + else + *v = (long)val; grib_context_log(a->context, GRIB_LOG_DEBUG, " Casting double %s to long", a->name); return GRIB_SUCCESS; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fb8dcc9a6..9940abddd 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -63,9 +63,11 @@ if( HAVE_BUILD_TOOLS ) grib_uerra grib_2nd_order_numValues grib_ecc-136 + grib_ecc-966 grib_ecc-967 - grib_ecc-1065 + grib_ecc-1150 grib_ecc-1053 + grib_ecc-1065 bufr_json_samples bufr_ecc-359 bufr_ecc-517 diff --git a/tests/grib_ecc-1150.sh b/tests/grib_ecc-1150.sh new file mode 100755 index 000000000..fa2de3367 --- /dev/null +++ b/tests/grib_ecc-1150.sh @@ -0,0 +1,41 @@ +#!/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.sh +set -u +# --------------------------------------------------------- +# This is the test for the JIRA issue ECC-1150 +# ECC-1150: keys 'lowerLimit' and 'upperLimit' cannot be MISSING +# --------------------------------------------------------- +label="grib_ecc-1150-test" +tempGrib=temp.${label}.grib +tempFilt=temp.${label}.filt + +in=$ECCODES_SAMPLES_PATH/GRIB2.tmpl +${tools_dir}/grib_set -s \ + productDefinitionTemplateNumber=5,scaleFactorOfLowerLimit=missing,scaledValueOfLowerLimit=missing \ + $in $tempGrib + +grib_check_key_equals $tempGrib lowerLimit MISSING +grib_check_key_equals $tempGrib upperLimit 0 + +cat > $tempFilt <$tempErrs +grep -q "ECCODES ERROR : unpack_double for radius" $tempErrs + +# Clean up +rm -f $tempGrib $tempErrs