From a918e9fa6eb93d865d1054e01d78d6d13b56032b Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Thu, 10 Oct 2019 18:21:05 +0100 Subject: [PATCH 01/23] ECC-992: GRIB encoding: Data Quality Checks (initial attempt) --- examples/C/large_grib1.c | 4 ++-- ...rib_accessor_class_data_g1simple_packing.c | 2 +- ...rib_accessor_class_data_g2simple_packing.c | 2 +- src/grib_accessor_class_data_simple_packing.c | 20 ++++++++++++---- src/grib_api_internal.h | 1 + src/grib_api_prototypes.h | 1 + src/grib_context.c | 23 +++++++++++-------- src/grib_util.c | 22 ++++++++++++++++++ tests/grib_encode_pthreads.c | 4 ++-- 9 files changed, 59 insertions(+), 20 deletions(-) diff --git a/examples/C/large_grib1.c b/examples/C/large_grib1.c index f631824c3..8fc11852e 100644 --- a/examples/C/large_grib1.c +++ b/examples/C/large_grib1.c @@ -29,8 +29,8 @@ int main() } for (i=0; icontext,GRIB_LOG_ERROR,"unable to compute packing parameters\n"); + grib_context_log(a->context,GRIB_LOG_ERROR,"GRIB1 simple packing: unable to set values"); return ret; } diff --git a/src/grib_accessor_class_data_g2simple_packing.c b/src/grib_accessor_class_data_g2simple_packing.c index 6748b965d..033b4848a 100644 --- a/src/grib_accessor_class_data_g2simple_packing.c +++ b/src/grib_accessor_class_data_g2simple_packing.c @@ -218,7 +218,7 @@ static int pack_double(grib_accessor* a, const double* cval, size_t *len) case GRIB_SUCCESS: break; default: - grib_context_log(a->context,GRIB_LOG_ERROR,"unable to compute packing parameters\n"); + grib_context_log(a->context,GRIB_LOG_ERROR,"GRIB2 simple packing: unable to set values"); return ret; } diff --git a/src/grib_accessor_class_data_simple_packing.c b/src/grib_accessor_class_data_simple_packing.c index f08c6c188..f872f8c4d 100644 --- a/src/grib_accessor_class_data_simple_packing.c +++ b/src/grib_accessor_class_data_simple_packing.c @@ -505,12 +505,22 @@ static int producing_large_constant_fields(const grib_context* c, grib_handle* h } #endif -static int check_range(const double val) +static int check_range(grib_handle* h, const double val) { + int result = GRIB_SUCCESS; + grib_context* ctx = h->context; if (val < DBL_MAX && val > -DBL_MAX) - return GRIB_SUCCESS; + result = GRIB_SUCCESS; else - return GRIB_ENCODING_ERROR; + result = GRIB_ENCODING_ERROR; + + /* Data Quality checks */ + if (ctx->grib_data_quality_checks && result == GRIB_SUCCESS) { + /*TODO: get limits for the current parameter*/ + result = grib_util_grib_data_quality_check(h, val); + } + + return result; } static int pack_double(grib_accessor* a, const double* val, size_t *len) @@ -577,11 +587,11 @@ static int pack_double(grib_accessor* a, const double* val, size_t *len) else if (val[i] < min ) min = val[i]; } #endif - if ((err = check_range(max)) != GRIB_SUCCESS) { + if ((err = check_range(gh, max)) != GRIB_SUCCESS) { grib_context_log(a->context,GRIB_LOG_ERROR,"Maximum value out of range: %g", max); return err; } - if ((err = check_range(min)) != GRIB_SUCCESS) { + if ((err = check_range(gh, min)) != GRIB_SUCCESS) { grib_context_log(a->context,GRIB_LOG_ERROR,"Minimum value out of range: %g", min); return err; } diff --git a/src/grib_api_internal.h b/src/grib_api_internal.h index 4e7ca8dd1..12adf1636 100644 --- a/src/grib_api_internal.h +++ b/src/grib_api_internal.h @@ -1053,6 +1053,7 @@ struct grib_context int bufrdc_mode; int bufr_set_to_missing_if_out_of_range; int bufr_multi_element_constant_arrays; + int grib_data_quality_checks; FILE* log_stream; grib_trie* classes; grib_trie* lists; diff --git a/src/grib_api_prototypes.h b/src/grib_api_prototypes.h index 07efcdc65..131cd8e28 100644 --- a/src/grib_api_prototypes.h +++ b/src/grib_api_prototypes.h @@ -1479,6 +1479,7 @@ int is_index_file(const char *filename); char get_dir_separator_char(void); char *codes_getenv(const char *name); size_t sum_of_pl_array(const long* pl, size_t plsize); +int grib_util_grib_data_quality_check(grib_handle* h, double val); /* bufr_util.c */ int compute_bufr_key_rank(grib_handle *h, grib_string_list *keys, const char *key); diff --git a/src/grib_context.c b/src/grib_context.c index da8b61d2e..e030776e2 100644 --- a/src/grib_context.c +++ b/src/grib_context.c @@ -308,13 +308,13 @@ static grib_context default_grib_context = { &default_seek, /* lfile seek procedure */ &default_feof, /* file feof procedure */ - &default_log, /* logging_procedure */ - &default_print, /* print procedure */ - 0, /* grib_codetable* */ - 0, /* grib_smart_table* */ - 0, /* char* outfilename */ - 0, /* int multi_support_on */ - 0, /* grib_multi_support* multi_support*/ + &default_log, /* output_log */ + &default_print, /* print */ + 0, /* codetable */ + 0, /* smart_table */ + 0, /* outfilename */ + 0, /* multi_support_on */ + 0, /* multi_support */ 0, /* grib_definition_files_dir */ 0, /* handle_file_count */ 0, /* handle_total_count */ @@ -323,9 +323,9 @@ static grib_context default_grib_context = { 0, /* gts_header_on */ 0, /* gribex_mode_on */ 0, /* large_constant_fields */ - 0, /* grib_itrie* keys */ + 0, /* keys */ 0, /* keys_count */ - 0, /* grib_itrie* concepts_index */ + 0, /* concepts_index */ 0, /* concepts_count */ {0,}, /* concepts */ 0, /* hash_array_index */ @@ -337,6 +337,7 @@ static grib_context default_grib_context = { 0, /* bufrdc_mode */ 0, /* bufr_set_to_missing_if_out_of_range */ 0, /* bufr_multi_element_constant_arrays */ + 0, /* grib_data_quality_checks */ 0, /* log_stream */ 0, /* classes */ 0, /* lists */ @@ -371,6 +372,7 @@ grib_context* grib_context_get_default() const char* bufrdc_mode = NULL; const char* bufr_set_to_missing_if_out_of_range = NULL; const char* bufr_multi_element_constant_arrays = NULL; + const char* grib_data_quality_checks = NULL; const char* file_pool_max_opened_files = NULL; #ifdef ENABLE_FLOATING_POINT_EXCEPTIONS @@ -381,6 +383,7 @@ grib_context* grib_context_get_default() bufrdc_mode = getenv("ECCODES_BUFRDC_MODE_ON"); bufr_set_to_missing_if_out_of_range = getenv("ECCODES_BUFR_SET_TO_MISSING_IF_OUT_OF_RANGE"); bufr_multi_element_constant_arrays = getenv("ECCODES_BUFR_MULTI_ELEMENT_CONSTANT_ARRAYS"); + grib_data_quality_checks = getenv("ECCODES_GRIB_DATA_QUALITY_CHECKS"); large_constant_fields = codes_getenv("ECCODES_GRIB_LARGE_CONSTANT_FIELDS"); no_abort = codes_getenv("ECCODES_NO_ABORT"); debug = codes_getenv("ECCODES_DEBUG"); @@ -479,6 +482,8 @@ grib_context* grib_context_get_default() atoi(bufr_set_to_missing_if_out_of_range) : 0; default_grib_context.bufr_multi_element_constant_arrays = bufr_multi_element_constant_arrays ? atoi(bufr_multi_element_constant_arrays) : 0; + default_grib_context.grib_data_quality_checks = grib_data_quality_checks ? + atoi(grib_data_quality_checks) : 0; default_grib_context.file_pool_max_opened_files = file_pool_max_opened_files ? atoi(file_pool_max_opened_files) : DEFAULT_FILE_POOL_MAX_OPENED_FILES; } diff --git a/src/grib_util.c b/src/grib_util.c index b5bd0db27..868d630aa 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2039,3 +2039,25 @@ size_t sum_of_pl_array(const long* pl, size_t plsize) } return count; } + +int grib_util_grib_data_quality_check(grib_handle* h, double val) +{ + /* TODO: From the paramId of the handle, get its limits. For now hardcoded */ + long paramId = 0; + char shortName[256]={0,}; + size_t len = sizeof(shortName); + + /*const double MIN_FIELD_VALUE_ALLOWED = -2e7;*/ + const double MAX_FIELD_VALUE_ALLOWED = +2e7; + + if (val > MAX_FIELD_VALUE_ALLOWED) { + if (grib_get_long(h, "paramId", ¶mId) == GRIB_SUCCESS && + grib_get_string(h,"shortName",shortName,&len) == GRIB_SUCCESS) + { + grib_context_log(h->context, GRIB_LOG_ERROR, "Parameter %d (%s): value %g exceeds the maximum limit of %g", + paramId, shortName, val, MAX_FIELD_VALUE_ALLOWED); + } + return GRIB_OUT_OF_RANGE; + } + return GRIB_SUCCESS; +} diff --git a/tests/grib_encode_pthreads.c b/tests/grib_encode_pthreads.c index 6c8472f76..7f0820477 100644 --- a/tests/grib_encode_pthreads.c +++ b/tests/grib_encode_pthreads.c @@ -76,11 +76,11 @@ static int encode_file(char *input_file, char *output_file) GRIB_CHECK(grib_get_size(clone_handle, "values", &values_len),0); values = (double*)malloc(values_len*sizeof(double)); - d=10e-8; + d=10e-10; e=d; count=1; for (i=0;i100) {e*=10; count=1;} + if (count>1000) {e*=2; count=1;} values[i]=d; d+=e; count++; From 935a881f42179854aab7c45fc4c8ccf3bb6ab24b Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Thu, 10 Oct 2019 18:30:55 +0100 Subject: [PATCH 02/23] ECC-992: Set upper limit to 1e6 --- src/grib_util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/grib_util.c b/src/grib_util.c index 868d630aa..c9c12a3aa 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2047,8 +2047,8 @@ int grib_util_grib_data_quality_check(grib_handle* h, double val) char shortName[256]={0,}; size_t len = sizeof(shortName); - /*const double MIN_FIELD_VALUE_ALLOWED = -2e7;*/ - const double MAX_FIELD_VALUE_ALLOWED = +2e7; + /* const double MIN_FIELD_VALUE_ALLOWED = -1e6; */ + const double MAX_FIELD_VALUE_ALLOWED = +1e6; if (val > MAX_FIELD_VALUE_ALLOWED) { if (grib_get_long(h, "paramId", ¶mId) == GRIB_SUCCESS && From 3f565b5c16a058ae24d782ae8bbbedc367a86b35 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Fri, 11 Oct 2019 10:54:18 +0100 Subject: [PATCH 03/23] Initialisation --- tests/bufr_ecc-604.c | 2 +- tests/grib_ecc-604.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bufr_ecc-604.c b/tests/bufr_ecc-604.c index cc2572ef2..a285729e2 100644 --- a/tests/bufr_ecc-604.c +++ b/tests/bufr_ecc-604.c @@ -19,7 +19,7 @@ int opt_write = 0; /* If 1 write handle to file */ static int encode_file(char *template_file, char *output_file) { - FILE *in, *out; + FILE *in, *out=NULL; codes_handle *source_handle = NULL; const void *buffer = NULL; size_t size = 0; diff --git a/tests/grib_ecc-604.c b/tests/grib_ecc-604.c index 4ae31c52a..136889e98 100644 --- a/tests/grib_ecc-604.c +++ b/tests/grib_ecc-604.c @@ -18,7 +18,7 @@ int opt_write = 0; /* If 1 write handle to file */ static int encode_file(char *template_file, char *output_file) { - FILE *in, *out; + FILE *in, *out=NULL; grib_handle *source_handle = NULL; const void *buffer = NULL; size_t size = 0; From a9009effdcdfdad2f2a8bd766649878ee013daef Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Fri, 11 Oct 2019 12:42:33 +0100 Subject: [PATCH 04/23] ECC-992: check min/max together --- src/grib_accessor_class_data_simple_packing.c | 27 ++++++++++--------- src/grib_api_prototypes.h | 2 +- src/grib_util.c | 24 +++++++---------- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/src/grib_accessor_class_data_simple_packing.c b/src/grib_accessor_class_data_simple_packing.c index f872f8c4d..91754db55 100644 --- a/src/grib_accessor_class_data_simple_packing.c +++ b/src/grib_accessor_class_data_simple_packing.c @@ -505,19 +505,24 @@ static int producing_large_constant_fields(const grib_context* c, grib_handle* h } #endif -static int check_range(grib_handle* h, const double val) +static int check_range(grib_handle* h, const double min_val, const double max_val) { int result = GRIB_SUCCESS; grib_context* ctx = h->context; - if (val < DBL_MAX && val > -DBL_MAX) - result = GRIB_SUCCESS; - else - result = GRIB_ENCODING_ERROR; + + if ( !(min_val < DBL_MAX && min_val > -DBL_MAX) ) { + grib_context_log(ctx, GRIB_LOG_ERROR, "Minimum value out of range: %g", min_val); + return GRIB_ENCODING_ERROR; + } + if ( !(max_val < DBL_MAX && max_val > -DBL_MAX) ) { + grib_context_log(ctx, GRIB_LOG_ERROR, "Maximum value out of range: %g", max_val); + return GRIB_ENCODING_ERROR; + } /* Data Quality checks */ - if (ctx->grib_data_quality_checks && result == GRIB_SUCCESS) { + if (ctx->grib_data_quality_checks) { /*TODO: get limits for the current parameter*/ - result = grib_util_grib_data_quality_check(h, val); + result = grib_util_grib_data_quality_check(h, min_val, max_val); } return result; @@ -587,12 +592,8 @@ static int pack_double(grib_accessor* a, const double* val, size_t *len) else if (val[i] < min ) min = val[i]; } #endif - if ((err = check_range(gh, max)) != GRIB_SUCCESS) { - grib_context_log(a->context,GRIB_LOG_ERROR,"Maximum value out of range: %g", max); - return err; - } - if ((err = check_range(gh, min)) != GRIB_SUCCESS) { - grib_context_log(a->context,GRIB_LOG_ERROR,"Minimum value out of range: %g", min); + if ((err = check_range(gh, min, max)) != GRIB_SUCCESS) { + //grib_context_log(a->context,GRIB_LOG_ERROR,"Maximum value out of range: %g", max); return err; } diff --git a/src/grib_api_prototypes.h b/src/grib_api_prototypes.h index 131cd8e28..a5d39f20c 100644 --- a/src/grib_api_prototypes.h +++ b/src/grib_api_prototypes.h @@ -1479,7 +1479,7 @@ int is_index_file(const char *filename); char get_dir_separator_char(void); char *codes_getenv(const char *name); size_t sum_of_pl_array(const long* pl, size_t plsize); -int grib_util_grib_data_quality_check(grib_handle* h, double val); +int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max_val); /* bufr_util.c */ int compute_bufr_key_rank(grib_handle *h, grib_string_list *keys, const char *key); diff --git a/src/grib_util.c b/src/grib_util.c index c9c12a3aa..252e7e5c6 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2040,24 +2040,20 @@ size_t sum_of_pl_array(const long* pl, size_t plsize) return count; } -int grib_util_grib_data_quality_check(grib_handle* h, double val) +int grib_util_grib_data_quality_check(grib_handle* h, const double min_val, const double max_val) { - /* TODO: From the paramId of the handle, get its limits. For now hardcoded */ - long paramId = 0; - char shortName[256]={0,}; - size_t len = sizeof(shortName); + /* TODO: The limits should depend on the paramId. For now hardcoded */ + static const double MIN_FIELD_VALUE_ALLOWED = -1e8; + static const double MAX_FIELD_VALUE_ALLOWED = +1e8; - /* const double MIN_FIELD_VALUE_ALLOWED = -1e6; */ - const double MAX_FIELD_VALUE_ALLOWED = +1e6; - - if (val > MAX_FIELD_VALUE_ALLOWED) { - if (grib_get_long(h, "paramId", ¶mId) == GRIB_SUCCESS && - grib_get_string(h,"shortName",shortName,&len) == GRIB_SUCCESS) - { - grib_context_log(h->context, GRIB_LOG_ERROR, "Parameter %d (%s): value %g exceeds the maximum limit of %g", - paramId, shortName, val, MAX_FIELD_VALUE_ALLOWED); + if (min_val < MIN_FIELD_VALUE_ALLOWED || max_val > MAX_FIELD_VALUE_ALLOWED) { + long paramId = 0; + if (grib_get_long(h, "paramId", ¶mId) == GRIB_SUCCESS) { + grib_context_log(h->context, GRIB_LOG_ERROR, "Parameter %ld: min/max (%g, %g) is outside limits (%g, %g)", + paramId, min_val, max_val, MIN_FIELD_VALUE_ALLOWED, MAX_FIELD_VALUE_ALLOWED); } return GRIB_OUT_OF_RANGE; } + return GRIB_SUCCESS; } From c34d820ab78f9b530e14fb99111e6fc3cf9b0fe9 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Fri, 11 Oct 2019 17:46:38 +0100 Subject: [PATCH 05/23] ECC-992: Added parameter limits definition file and new keys --- definitions/boot.def | 5 ++++ definitions/param_limits.def | 11 +++++++++ src/grib_accessor_class_data_simple_packing.c | 1 - src/grib_util.c | 24 +++++++++++++------ 4 files changed, 33 insertions(+), 8 deletions(-) create mode 100644 definitions/param_limits.def diff --git a/definitions/boot.def b/definitions/boot.def index 15ff30351..9b281ac23 100644 --- a/definitions/boot.def +++ b/definitions/boot.def @@ -15,6 +15,11 @@ UseEcmfConventions = getenv("ECCODES_USE_ECMF_CONVENTIONS","1") :hidden ; constant defaultTypeOfLevel="unknown" : hidden; +gribDataQualityChecks = getenv("ECCODES_GRIB_DATA_QUALITY_CHECKS","0") : hidden; +if (gribDataQualityChecks) { + template LIMITS "param_limits.def"; +} + # GRIBEX special boustrophedonic mode. See GRIB-472 # If the environment variable is not defined, the key will be 0 GRIBEX_boustrophedonic = getenv("ECCODES_GRIBEX_BOUSTROPHEDONIC","0") :hidden; diff --git a/definitions/param_limits.def b/definitions/param_limits.def new file mode 100644 index 000000000..01ee9e3de --- /dev/null +++ b/definitions/param_limits.def @@ -0,0 +1,11 @@ +constant default_min_val = -1e8 : long_type, hidden; +constant default_max_val = +1e8 : long_type, hidden; + +concept param_value_min(default_min_val) { + 0 = { paramId=130; } + 50 = { paramId=167; } +} : long_type, hidden; +concept param_value_max(default_max_val) { + 400 = { paramId=130; } + 373 = { paramId=167; } +} : long_type, hidden; diff --git a/src/grib_accessor_class_data_simple_packing.c b/src/grib_accessor_class_data_simple_packing.c index 91754db55..ad72d1ac0 100644 --- a/src/grib_accessor_class_data_simple_packing.c +++ b/src/grib_accessor_class_data_simple_packing.c @@ -521,7 +521,6 @@ static int check_range(grib_handle* h, const double min_val, const double max_va /* Data Quality checks */ if (ctx->grib_data_quality_checks) { - /*TODO: get limits for the current parameter*/ result = grib_util_grib_data_quality_check(h, min_val, max_val); } diff --git a/src/grib_util.c b/src/grib_util.c index 252e7e5c6..e7df3f99b 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2040,17 +2040,27 @@ size_t sum_of_pl_array(const long* pl, size_t plsize) return count; } -int grib_util_grib_data_quality_check(grib_handle* h, const double min_val, const double max_val) +int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max_val) { - /* TODO: The limits should depend on the paramId. For now hardcoded */ - static const double MIN_FIELD_VALUE_ALLOWED = -1e8; - static const double MAX_FIELD_VALUE_ALLOWED = +1e8; + int err = 0; + long min_field_value_allowed=0, max_field_value_allowed=0; + double dmin_allowed=0, dmax_allowed=0; - if (min_val < MIN_FIELD_VALUE_ALLOWED || max_val > MAX_FIELD_VALUE_ALLOWED) { + /* The limit keys must exist if we are here */ + err = grib_get_long(h, "param_value_min", &min_field_value_allowed); + if (err) return err; + err = grib_get_long(h, "param_value_max", &max_field_value_allowed); + if (err) return err; + + dmin_allowed = (double)min_field_value_allowed; + dmax_allowed = (double)max_field_value_allowed; + + if (min_val < dmin_allowed || max_val > dmax_allowed) { long paramId = 0; if (grib_get_long(h, "paramId", ¶mId) == GRIB_SUCCESS) { - grib_context_log(h->context, GRIB_LOG_ERROR, "Parameter %ld: min/max (%g, %g) is outside limits (%g, %g)", - paramId, min_val, max_val, MIN_FIELD_VALUE_ALLOWED, MAX_FIELD_VALUE_ALLOWED); + grib_context_log(h->context, GRIB_LOG_ERROR, + "Parameter %ld: min/max (%g, %g) is outside allowable limits (%g, %g)", + paramId, min_val, max_val, dmin_allowed, dmax_allowed); } return GRIB_OUT_OF_RANGE; } From 77c7a95a5a289c90d90441baaed410aed9f83cc6 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Fri, 11 Oct 2019 18:22:13 +0100 Subject: [PATCH 06/23] ECC-992: tune limits so tests pass --- definitions/param_limits.def | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/definitions/param_limits.def b/definitions/param_limits.def index 01ee9e3de..24616f996 100644 --- a/definitions/param_limits.def +++ b/definitions/param_limits.def @@ -1,11 +1,9 @@ -constant default_min_val = -1e8 : long_type, hidden; -constant default_max_val = +1e8 : long_type, hidden; +constant default_min_val = -1e9 : long_type, hidden; +constant default_max_val = +1e9 : long_type, hidden; concept param_value_min(default_min_val) { - 0 = { paramId=130; } - 50 = { paramId=167; } + 0 = { paramId=167; } } : long_type, hidden; concept param_value_max(default_max_val) { - 400 = { paramId=130; } 373 = { paramId=167; } } : long_type, hidden; From ad1810fcaf5596167ea2be9353f1d270d75940ed Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Tue, 26 Nov 2019 18:20:14 +0000 Subject: [PATCH 07/23] Remove the error message. In IFS 1000s of messages are posted. Need to track the guilty party --- src/grib_accessor_class_ascii.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/grib_accessor_class_ascii.c b/src/grib_accessor_class_ascii.c index 6ec67e107..10207e0ed 100644 --- a/src/grib_accessor_class_ascii.c +++ b/src/grib_accessor_class_ascii.c @@ -256,7 +256,7 @@ static int unpack_double (grib_accessor* a, double*v, size_t *len) return GRIB_SUCCESS; } - grib_context_log(a->context,GRIB_LOG_ERROR,"Cannot unpack %s as double. Hint: Try unpacking as string",a->name); + grib_context_log(a->context,GRIB_LOG_WARNING,"Cannot unpack %s as double. Hint: Try unpacking as string",a->name); return GRIB_NOT_IMPLEMENTED; } From 14f368f2a9e309ad6fc69ecd41bcf8092a45c1c8 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Wed, 27 Nov 2019 17:10:17 +0000 Subject: [PATCH 08/23] More error reporting --- src/grib_accessor_class_data_g1simple_packing.c | 2 +- src/grib_accessor_class_data_g2simple_packing.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/grib_accessor_class_data_g1simple_packing.c b/src/grib_accessor_class_data_g1simple_packing.c index 0dd23b033..33e4def97 100644 --- a/src/grib_accessor_class_data_g1simple_packing.c +++ b/src/grib_accessor_class_data_g1simple_packing.c @@ -283,7 +283,7 @@ static int pack_double(grib_accessor* a, const double* cval, size_t *len) case GRIB_SUCCESS: break; default: - grib_context_log(a->context,GRIB_LOG_ERROR,"GRIB1 simple packing: unable to set values"); + grib_context_log(a->context,GRIB_LOG_ERROR,"GRIB1 simple packing: unable to set values (%s)", grib_get_error_message(ret)); return ret; } diff --git a/src/grib_accessor_class_data_g2simple_packing.c b/src/grib_accessor_class_data_g2simple_packing.c index 033b4848a..80283cc5f 100644 --- a/src/grib_accessor_class_data_g2simple_packing.c +++ b/src/grib_accessor_class_data_g2simple_packing.c @@ -218,7 +218,7 @@ static int pack_double(grib_accessor* a, const double* cval, size_t *len) case GRIB_SUCCESS: break; default: - grib_context_log(a->context,GRIB_LOG_ERROR,"GRIB2 simple packing: unable to set values"); + grib_context_log(a->context,GRIB_LOG_ERROR,"GRIB2 simple packing: unable to set values (%s)", grib_get_error_message(ret)); return ret; } From 8f8c5393200ba20d0261be01232de4d958673811 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Wed, 27 Nov 2019 17:51:02 +0000 Subject: [PATCH 09/23] Quality checks: More error reporting --- src/grib_util.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/grib_util.c b/src/grib_util.c index e7df3f99b..9ca7bf58c 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2048,9 +2048,15 @@ int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max /* The limit keys must exist if we are here */ err = grib_get_long(h, "param_value_min", &min_field_value_allowed); - if (err) return err; + if (err) { + grib_context_log(h->context, GRIB_LOG_ERROR,"grib_util_grib_data_quality_check: Could not get param_value_min"); + return err; + } err = grib_get_long(h, "param_value_max", &max_field_value_allowed); - if (err) return err; + if (err) { + grib_context_log(h->context, GRIB_LOG_ERROR,"grib_util_grib_data_quality_check: Could not get param_value_max"); + return err; + } dmin_allowed = (double)min_field_value_allowed; dmax_allowed = (double)max_field_value_allowed; From 1e59d51c32ccfaad547ebf88f15eddad5e637207 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Wed, 27 Nov 2019 18:11:57 +0000 Subject: [PATCH 10/23] Quality checks: Add test --- src/grib_util.c | 4 +-- tests/CMakeLists.txt | 1 + tests/grib_data_quality_checks.sh | 48 +++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100755 tests/grib_data_quality_checks.sh diff --git a/src/grib_util.c b/src/grib_util.c index 9ca7bf58c..548392650 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2049,12 +2049,12 @@ int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max /* The limit keys must exist if we are here */ err = grib_get_long(h, "param_value_min", &min_field_value_allowed); if (err) { - grib_context_log(h->context, GRIB_LOG_ERROR,"grib_util_grib_data_quality_check: Could not get param_value_min"); + grib_context_log(h->context, GRIB_LOG_ERROR,"grib_data_quality_check: Could not get param_value_min"); return err; } err = grib_get_long(h, "param_value_max", &max_field_value_allowed); if (err) { - grib_context_log(h->context, GRIB_LOG_ERROR,"grib_util_grib_data_quality_check: Could not get param_value_max"); + grib_context_log(h->context, GRIB_LOG_ERROR,"grib_data_quality_check: Could not get param_value_max"); return err; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f62ad4a13..cb05e72ef 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -68,6 +68,7 @@ list( APPEND tests_no_data_reqd ) # These tests do require data downloads list( APPEND tests_data_reqd + grib_data_quality_checks bpv_limit grib_double_cmp grib_change_packing diff --git a/tests/grib_data_quality_checks.sh b/tests/grib_data_quality_checks.sh new file mode 100755 index 000000000..5e7c9e2d1 --- /dev/null +++ b/tests/grib_data_quality_checks.sh @@ -0,0 +1,48 @@ +#!/bin/sh +# Copyright 2005-2019 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 data quality checks +# --------------------------------------------------------- +label="grib_data_quality" +tempOut=temp.${label}.out +tempErr=temp.${label}.err + +input1=${data_dir}/reduced_gaussian_surface.grib1 +input2=${data_dir}/reduced_gaussian_surface.grib2 + +# Data quality checks disabled. Create cause huge values for temperature +unset ECCODES_GRIB_DATA_QUALITY_CHECKS +${tools_dir}/grib_set -s scaleValuesBy=100 $input1 $tempOut +${tools_dir}/grib_set -s scaleValuesBy=100 $input2 $tempOut + +# Data quality checks enabled. Commands should fail +export ECCODES_GRIB_DATA_QUALITY_CHECKS=1 +set +e +${tools_dir}/grib_set -s scaleValuesBy=100 $input1 $tempOut 2>$tempErr +status=$? +set -e +[ $status -ne 0 ] +grep -q 'GRIB1 simple packing: unable to set values' $tempErr +grep -q 'outside allowable limits' $tempErr + +set +e +${tools_dir}/grib_set -s scaleValuesBy=100 $input2 $tempOut 2>$tempErr +status=$? +set -e +[ $status -ne 0 ] +grep -q 'GRIB2 simple packing: unable to set values' $tempErr +grep -q 'outside allowable limits' $tempErr + + +# Clean up +rm -f $tempOut $tempErr From ebcd2168f76ea5aa0c63fd31df4b956f74038613 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Thu, 28 Nov 2019 12:30:38 +0000 Subject: [PATCH 11/23] Use of ecc_snprintf --- src/grib_api_internal.h | 3 +++ src/grib_jasper_encoding.c | 6 +----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/grib_api_internal.h b/src/grib_api_internal.h index 12adf1636..5028d7ff3 100644 --- a/src/grib_api_internal.h +++ b/src/grib_api_internal.h @@ -64,6 +64,7 @@ #include #include #include + #define ecc_snprintf snprintf #else #include #include @@ -99,6 +100,8 @@ # define strdup(str) _strdup(str) # endif + #define ecc_snprintf _snprintf + #endif diff --git a/src/grib_jasper_encoding.c b/src/grib_jasper_encoding.c index a3de941c6..45a2d2ba0 100644 --- a/src/grib_jasper_encoding.c +++ b/src/grib_jasper_encoding.c @@ -161,11 +161,7 @@ int grib_jasper_encode(grib_context *c, j2k_encode_helper *helper) if( helper->compression != 0) { /* Lossy */ -#ifndef ECCODES_ON_WINDOWS - snprintf (opts, MAXOPTSSIZE, "mode=real\nrate=%f", 1.0/helper->compression); -#else - _snprintf(opts, MAXOPTSSIZE, "mode=real\nrate=%f", 1.0/helper->compression); -#endif + ecc_snprintf(opts, MAXOPTSSIZE, "mode=real\nrate=%f", 1.0/helper->compression); } Assert(cmpt.width_ * cmpt.height_ * cmpt.cps_ == buflen); From beba803afb5653361fb84834933ab2e9720fea8e Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Thu, 28 Nov 2019 15:53:46 +0000 Subject: [PATCH 12/23] ECC-992: New environment variable ECCODES_DEFINITION_PATH_SUPPLEMENT. Also added test --- src/grib_context.c | 10 ++++++++++ tests/grib_data_quality_checks.sh | 28 +++++++++++++++++++++++++++- tools/codes_info.c | 6 +++--- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/grib_context.c b/src/grib_context.c index e030776e2..e4a0aab5e 100644 --- a/src/grib_context.c +++ b/src/grib_context.c @@ -461,6 +461,16 @@ grib_context* grib_context_get_default() } } + /* Definitions path supplement: Added at the head of existing path */ + { + const char* defs_supp = codes_getenv("ECCODES_DEFINITION_PATH_SUPPLEMENT"); + if (defs_supp) { + char buffer[DEF_PATH_MAXLEN]; + ecc_snprintf(buffer, DEF_PATH_MAXLEN, "%s:%s", defs_supp, default_grib_context.grib_definition_files_path); + default_grib_context.grib_definition_files_path = strdup(buffer); + } + } + grib_context_log(&default_grib_context, GRIB_LOG_DEBUG, "Definitions path: %s", default_grib_context.grib_definition_files_path); grib_context_log(&default_grib_context, GRIB_LOG_DEBUG, "Samples path: %s", diff --git a/tests/grib_data_quality_checks.sh b/tests/grib_data_quality_checks.sh index 5e7c9e2d1..b1bb3a558 100755 --- a/tests/grib_data_quality_checks.sh +++ b/tests/grib_data_quality_checks.sh @@ -11,7 +11,7 @@ . ./include.sh set -u # --------------------------------------------------------- -# This is the test for data quality checks +# Tests for data quality checks # --------------------------------------------------------- label="grib_data_quality" tempOut=temp.${label}.out @@ -19,6 +19,8 @@ tempErr=temp.${label}.err input1=${data_dir}/reduced_gaussian_surface.grib1 input2=${data_dir}/reduced_gaussian_surface.grib2 +grib_check_key_equals $input1 paramId 167 +grib_check_key_equals $input2 paramId 167 # Data quality checks disabled. Create cause huge values for temperature unset ECCODES_GRIB_DATA_QUALITY_CHECKS @@ -43,6 +45,30 @@ set -e grep -q 'GRIB2 simple packing: unable to set values' $tempErr grep -q 'outside allowable limits' $tempErr +# Override the defaults +# ---------------------- +tempDir=tempdir.$label +rm -rf $tempDir +mkdir -p $tempDir +# Set a large limit for temperature +cat > $tempDir/param_limits.def <grib_definition_files_path); printf("Definition files path can be changed by setting ECCODES_DEFINITION_PATH environment variable\n"); } printf("\n"); @@ -118,7 +118,7 @@ int main( int argc,char* argv[]) if ((path=codes_getenv("ECCODES_DEFINITION_PATH")) != NULL) { printf("%s",path); } else { - printf("%s",ECCODES_DEFINITION_PATH); + printf("%s",context->grib_definition_files_path); } } From 696d08785f04829578685618befe0c0a6be14c41 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Fri, 29 Nov 2019 12:39:28 +0000 Subject: [PATCH 13/23] Fix MEMFS build: remove empty tables --- definitions/grib2/tables/0/4.2.table | 0 definitions/grib2/tables/1/4.2.table | 0 definitions/grib2/tables/10/4.2.table | 5 ----- definitions/grib2/tables/2/4.2.table | 5 ----- definitions/grib2/tables/3/4.2.table | 5 ----- definitions/grib2/tables/4/4.2.table | 5 ----- definitions/grib2/tables/5/4.2.table | 5 ----- definitions/grib2/tables/6/4.2.table | 5 ----- definitions/grib2/tables/7/4.2.table | 5 ----- definitions/grib2/tables/8/4.2.table | 5 ----- definitions/grib2/tables/9/4.2.table | 5 ----- 11 files changed, 45 deletions(-) delete mode 100644 definitions/grib2/tables/0/4.2.table delete mode 100644 definitions/grib2/tables/1/4.2.table delete mode 100644 definitions/grib2/tables/10/4.2.table delete mode 100644 definitions/grib2/tables/2/4.2.table delete mode 100644 definitions/grib2/tables/3/4.2.table delete mode 100644 definitions/grib2/tables/4/4.2.table delete mode 100644 definitions/grib2/tables/5/4.2.table delete mode 100644 definitions/grib2/tables/6/4.2.table delete mode 100644 definitions/grib2/tables/7/4.2.table delete mode 100644 definitions/grib2/tables/8/4.2.table delete mode 100644 definitions/grib2/tables/9/4.2.table diff --git a/definitions/grib2/tables/0/4.2.table b/definitions/grib2/tables/0/4.2.table deleted file mode 100644 index e69de29bb..000000000 diff --git a/definitions/grib2/tables/1/4.2.table b/definitions/grib2/tables/1/4.2.table deleted file mode 100644 index e69de29bb..000000000 diff --git a/definitions/grib2/tables/10/4.2.table b/definitions/grib2/tables/10/4.2.table deleted file mode 100644 index ff9553645..000000000 --- a/definitions/grib2/tables/10/4.2.table +++ /dev/null @@ -1,5 +0,0 @@ -# CODE TABLE 4.2, Parameter number by product discipline and parameter category -# 4 4 unknown -# 151 151 unknown -# 192 192 unknown -# 255 255 Missing diff --git a/definitions/grib2/tables/2/4.2.table b/definitions/grib2/tables/2/4.2.table deleted file mode 100644 index ff9553645..000000000 --- a/definitions/grib2/tables/2/4.2.table +++ /dev/null @@ -1,5 +0,0 @@ -# CODE TABLE 4.2, Parameter number by product discipline and parameter category -# 4 4 unknown -# 151 151 unknown -# 192 192 unknown -# 255 255 Missing diff --git a/definitions/grib2/tables/3/4.2.table b/definitions/grib2/tables/3/4.2.table deleted file mode 100644 index ff9553645..000000000 --- a/definitions/grib2/tables/3/4.2.table +++ /dev/null @@ -1,5 +0,0 @@ -# CODE TABLE 4.2, Parameter number by product discipline and parameter category -# 4 4 unknown -# 151 151 unknown -# 192 192 unknown -# 255 255 Missing diff --git a/definitions/grib2/tables/4/4.2.table b/definitions/grib2/tables/4/4.2.table deleted file mode 100644 index ff9553645..000000000 --- a/definitions/grib2/tables/4/4.2.table +++ /dev/null @@ -1,5 +0,0 @@ -# CODE TABLE 4.2, Parameter number by product discipline and parameter category -# 4 4 unknown -# 151 151 unknown -# 192 192 unknown -# 255 255 Missing diff --git a/definitions/grib2/tables/5/4.2.table b/definitions/grib2/tables/5/4.2.table deleted file mode 100644 index ff9553645..000000000 --- a/definitions/grib2/tables/5/4.2.table +++ /dev/null @@ -1,5 +0,0 @@ -# CODE TABLE 4.2, Parameter number by product discipline and parameter category -# 4 4 unknown -# 151 151 unknown -# 192 192 unknown -# 255 255 Missing diff --git a/definitions/grib2/tables/6/4.2.table b/definitions/grib2/tables/6/4.2.table deleted file mode 100644 index ff9553645..000000000 --- a/definitions/grib2/tables/6/4.2.table +++ /dev/null @@ -1,5 +0,0 @@ -# CODE TABLE 4.2, Parameter number by product discipline and parameter category -# 4 4 unknown -# 151 151 unknown -# 192 192 unknown -# 255 255 Missing diff --git a/definitions/grib2/tables/7/4.2.table b/definitions/grib2/tables/7/4.2.table deleted file mode 100644 index ff9553645..000000000 --- a/definitions/grib2/tables/7/4.2.table +++ /dev/null @@ -1,5 +0,0 @@ -# CODE TABLE 4.2, Parameter number by product discipline and parameter category -# 4 4 unknown -# 151 151 unknown -# 192 192 unknown -# 255 255 Missing diff --git a/definitions/grib2/tables/8/4.2.table b/definitions/grib2/tables/8/4.2.table deleted file mode 100644 index ff9553645..000000000 --- a/definitions/grib2/tables/8/4.2.table +++ /dev/null @@ -1,5 +0,0 @@ -# CODE TABLE 4.2, Parameter number by product discipline and parameter category -# 4 4 unknown -# 151 151 unknown -# 192 192 unknown -# 255 255 Missing diff --git a/definitions/grib2/tables/9/4.2.table b/definitions/grib2/tables/9/4.2.table deleted file mode 100644 index ff9553645..000000000 --- a/definitions/grib2/tables/9/4.2.table +++ /dev/null @@ -1,5 +0,0 @@ -# CODE TABLE 4.2, Parameter number by product discipline and parameter category -# 4 4 unknown -# 151 151 unknown -# 192 192 unknown -# 255 255 Missing From 089604ec0352f29ffd953d4f139ce289abd0a1b5 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Fri, 29 Nov 2019 14:45:55 +0000 Subject: [PATCH 14/23] ECC-992: Renamed environment variable --- src/grib_context.c | 8 ++++---- tests/grib_data_quality_checks.sh | 26 ++++++++++++++++++++------ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/grib_context.c b/src/grib_context.c index e4a0aab5e..e2616c08d 100644 --- a/src/grib_context.c +++ b/src/grib_context.c @@ -461,12 +461,12 @@ grib_context* grib_context_get_default() } } - /* Definitions path supplement: Added at the head of existing path */ + /* Definitions path extra: Added at the head of (i.e. before) existing path */ { - const char* defs_supp = codes_getenv("ECCODES_DEFINITION_PATH_SUPPLEMENT"); - if (defs_supp) { + const char* defs_extra = getenv("ECCODES_EXTRA_DEFINITION_PATH"); + if (defs_extra) { char buffer[DEF_PATH_MAXLEN]; - ecc_snprintf(buffer, DEF_PATH_MAXLEN, "%s:%s", defs_supp, default_grib_context.grib_definition_files_path); + ecc_snprintf(buffer, DEF_PATH_MAXLEN, "%s:%s", defs_extra, default_grib_context.grib_definition_files_path); default_grib_context.grib_definition_files_path = strdup(buffer); } } diff --git a/tests/grib_data_quality_checks.sh b/tests/grib_data_quality_checks.sh index b1bb3a558..05f1e1cac 100755 --- a/tests/grib_data_quality_checks.sh +++ b/tests/grib_data_quality_checks.sh @@ -14,20 +14,34 @@ set -u # Tests for data quality checks # --------------------------------------------------------- label="grib_data_quality" -tempOut=temp.${label}.out +tempOut=temp.1.${label}.out +temp2=temp.2.${label}.out tempErr=temp.${label}.err +# Start with clean environment +unset ECCODES_GRIB_DATA_QUALITY_CHECKS +unset ECCODES_EXTRA_DEFINITION_PATH + + input1=${data_dir}/reduced_gaussian_surface.grib1 input2=${data_dir}/reduced_gaussian_surface.grib2 grib_check_key_equals $input1 paramId 167 grib_check_key_equals $input2 paramId 167 -# Data quality checks disabled. Create cause huge values for temperature -unset ECCODES_GRIB_DATA_QUALITY_CHECKS +# Data quality checks disabled. Create huge values for temperature ${tools_dir}/grib_set -s scaleValuesBy=100 $input1 $tempOut ${tools_dir}/grib_set -s scaleValuesBy=100 $input2 $tempOut -# Data quality checks enabled. Commands should fail +# Data quality checks enabled. Repacking should fail +export ECCODES_GRIB_DATA_QUALITY_CHECKS=1 +set +e +${tools_dir}/grib_copy -r $tempOut /dev/null 2>$tempErr +status=$? +set -e +[ $status -ne 0 ] +grep -q 'outside allowable limits' $tempErr + +# Data quality checks enabled. Scaling should fail export ECCODES_GRIB_DATA_QUALITY_CHECKS=1 set +e ${tools_dir}/grib_set -s scaleValuesBy=100 $input1 $tempOut 2>$tempErr @@ -62,9 +76,9 @@ cat > $tempDir/param_limits.def < Date: Fri, 29 Nov 2019 16:52:06 +0000 Subject: [PATCH 15/23] ECC-992: Implement warning mode (ECCODES_GRIB_DATA_QUALITY_CHECKS=2) --- src/grib_util.c | 21 +++++++++++++++------ tests/grib_data_quality_checks.sh | 8 ++++++++ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/grib_util.c b/src/grib_util.c index fda2f3510..0fba4b86a 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2113,16 +2113,24 @@ int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max int err = 0; long min_field_value_allowed=0, max_field_value_allowed=0; double dmin_allowed=0, dmax_allowed=0; + grib_context* ctx = h->context; + int is_error = 1; + /* + * If grib_data_quality_checks == 1, limits failure results in an error + * If grib_data_quality_checks == 2, limits failure results in a warning + */ + Assert( ctx->grib_data_quality_checks == 1 || ctx->grib_data_quality_checks == 2 ); + is_error = (ctx->grib_data_quality_checks == 1); /* The limit keys must exist if we are here */ err = grib_get_long(h, "param_value_min", &min_field_value_allowed); if (err) { - grib_context_log(h->context, GRIB_LOG_ERROR,"grib_data_quality_check: Could not get param_value_min"); + grib_context_log(ctx, GRIB_LOG_ERROR,"grib_data_quality_check: Could not get param_value_min"); return err; } err = grib_get_long(h, "param_value_max", &max_field_value_allowed); if (err) { - grib_context_log(h->context, GRIB_LOG_ERROR,"grib_data_quality_check: Could not get param_value_max"); + grib_context_log(ctx, GRIB_LOG_ERROR,"grib_data_quality_check: Could not get param_value_max"); return err; } @@ -2132,11 +2140,12 @@ int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max if (min_val < dmin_allowed || max_val > dmax_allowed) { long paramId = 0; if (grib_get_long(h, "paramId", ¶mId) == GRIB_SUCCESS) { - grib_context_log(h->context, GRIB_LOG_ERROR, - "Parameter %ld: min/max (%g, %g) is outside allowable limits (%g, %g)", - paramId, min_val, max_val, dmin_allowed, dmax_allowed); + fprintf(stderr, "ECCODES %s : Parameter %ld: min/max (%g, %g) is outside allowable limits (%g, %g)\n", + (is_error? "ERROR":"WARNING"), paramId, min_val, max_val, dmin_allowed, dmax_allowed); + } + if (is_error) { + return GRIB_OUT_OF_RANGE; /* Failure */ } - return GRIB_OUT_OF_RANGE; } return GRIB_SUCCESS; diff --git a/tests/grib_data_quality_checks.sh b/tests/grib_data_quality_checks.sh index 05f1e1cac..c76574f34 100755 --- a/tests/grib_data_quality_checks.sh +++ b/tests/grib_data_quality_checks.sh @@ -41,6 +41,13 @@ set -e [ $status -ne 0 ] grep -q 'outside allowable limits' $tempErr + +# Data quality checks enabled but only as a warning. Repacking should pass +export ECCODES_GRIB_DATA_QUALITY_CHECKS=2 +${tools_dir}/grib_copy -r $tempOut /dev/null 2>$tempErr +grep -q 'outside allowable limits' $tempErr + + # Data quality checks enabled. Scaling should fail export ECCODES_GRIB_DATA_QUALITY_CHECKS=1 set +e @@ -59,6 +66,7 @@ set -e grep -q 'GRIB2 simple packing: unable to set values' $tempErr grep -q 'outside allowable limits' $tempErr + # Override the defaults # ---------------------- tempDir=tempdir.$label From a1f28f4bafda60770c991bd0c93b9a19da98f235 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Sat, 30 Nov 2019 13:32:48 +0000 Subject: [PATCH 16/23] ECC-992: Print details of matching concept --- src/grib_util.c | 58 ++++++++++++++++++++++++++++--- tests/grib_data_quality_checks.sh | 8 ++--- 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/src/grib_util.c b/src/grib_util.c index 0fba4b86a..bed06e5e5 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2108,6 +2108,44 @@ size_t sum_of_pl_array(const long* pl, size_t plsize) return count; } +static int get_concept_condition_string(grib_handle* h, const char* key, char* result) +{ + int err = 0; + int length = 0; + char strVal[32]={0,}; + size_t len = sizeof(strVal); + grib_concept_value* concept_value = NULL; + grib_accessor* acc = grib_find_accessor(h, key); + if (!acc) return GRIB_NOT_FOUND; + + err = grib_get_string(h, key, strVal,&len); + if (err) return GRIB_INTERNAL_ERROR; + + concept_value = action_concept_get_concept(acc); + while (concept_value) { + int err = 0; + long lres = 0; + grib_concept_condition* concept_condition = concept_value->conditions; + + if (strcmp(strVal, concept_value->name)==0) { + while (concept_condition) { + grib_expression* expression = concept_condition->expression; + Assert(expression); + err = grib_expression_evaluate_long(h,expression,&lres); + if (err) return err; + length += sprintf(result+length, "%s%s=%ld", + (length==0?"":","),concept_condition->name, lres); + + concept_condition = concept_condition->next; + } + } + + concept_value = concept_value->next; + } + if (length == 0) return GRIB_CONCEPT_NO_MATCH; + return GRIB_SUCCESS; +} + int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max_val) { int err = 0; @@ -2137,11 +2175,21 @@ int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max dmin_allowed = (double)min_field_value_allowed; dmax_allowed = (double)max_field_value_allowed; - if (min_val < dmin_allowed || max_val > dmax_allowed) { - long paramId = 0; - if (grib_get_long(h, "paramId", ¶mId) == GRIB_SUCCESS) { - fprintf(stderr, "ECCODES %s : Parameter %ld: min/max (%g, %g) is outside allowable limits (%g, %g)\n", - (is_error? "ERROR":"WARNING"), paramId, min_val, max_val, dmin_allowed, dmax_allowed); + if (min_val < dmin_allowed) { + char description[1024] = {0,}; + if (get_concept_condition_string(h, "param_value_min", description)==GRIB_SUCCESS) { + fprintf(stderr, "ECCODES %s : (%s): minimum (%g) is less than the allowable limit (%g)\n", + (is_error? "ERROR":"WARNING"), description, min_val, dmin_allowed); + } + if (is_error) { + return GRIB_OUT_OF_RANGE; /* Failure */ + } + } + if (max_val > dmax_allowed) { + char description[1024] = {0,}; + if (get_concept_condition_string(h, "param_value_max", description)==GRIB_SUCCESS) { + fprintf(stderr, "ECCODES %s : (%s): maximum (%g) is more than the allowable limit (%g)\n", + (is_error? "ERROR":"WARNING"), description, max_val, dmax_allowed); } if (is_error) { return GRIB_OUT_OF_RANGE; /* Failure */ diff --git a/tests/grib_data_quality_checks.sh b/tests/grib_data_quality_checks.sh index c76574f34..65a08ce79 100755 --- a/tests/grib_data_quality_checks.sh +++ b/tests/grib_data_quality_checks.sh @@ -39,13 +39,13 @@ ${tools_dir}/grib_copy -r $tempOut /dev/null 2>$tempErr status=$? set -e [ $status -ne 0 ] -grep -q 'outside allowable limits' $tempErr +grep -q 'more than the allowable limit' $tempErr # Data quality checks enabled but only as a warning. Repacking should pass export ECCODES_GRIB_DATA_QUALITY_CHECKS=2 ${tools_dir}/grib_copy -r $tempOut /dev/null 2>$tempErr -grep -q 'outside allowable limits' $tempErr +grep -q 'more than the allowable limit' $tempErr # Data quality checks enabled. Scaling should fail @@ -56,7 +56,7 @@ status=$? set -e [ $status -ne 0 ] grep -q 'GRIB1 simple packing: unable to set values' $tempErr -grep -q 'outside allowable limits' $tempErr +grep -q 'allowable limit' $tempErr set +e ${tools_dir}/grib_set -s scaleValuesBy=100 $input2 $tempOut 2>$tempErr @@ -64,7 +64,7 @@ status=$? set -e [ $status -ne 0 ] grep -q 'GRIB2 simple packing: unable to set values' $tempErr -grep -q 'outside allowable limits' $tempErr +grep -q 'allowable limit' $tempErr # Override the defaults From 58e7d587072da8d221c1c0aefe1b76d42d52da7f Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Sun, 1 Dec 2019 16:07:01 +0000 Subject: [PATCH 17/23] Fix comments --- src/grib_accessor_class_data_simple_packing.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/grib_accessor_class_data_simple_packing.c b/src/grib_accessor_class_data_simple_packing.c index ad72d1ac0..50e7a7695 100644 --- a/src/grib_accessor_class_data_simple_packing.c +++ b/src/grib_accessor_class_data_simple_packing.c @@ -592,7 +592,6 @@ static int pack_double(grib_accessor* a, const double* val, size_t *len) } #endif if ((err = check_range(gh, min, max)) != GRIB_SUCCESS) { - //grib_context_log(a->context,GRIB_LOG_ERROR,"Maximum value out of range: %g", max); return err; } From e9045909f351fab82dfdbcb992a8263592b3677d Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Sun, 1 Dec 2019 16:13:38 +0000 Subject: [PATCH 18/23] New function get_concept_condition_string and unit tests for it --- src/action_class_concept.c | 42 ++++++++++++++++++++++++++++++++++++++ src/grib_api_prototypes.h | 1 + src/grib_util.c | 38 ---------------------------------- tests/unit_tests.c | 22 ++++++++++++++++++++ 4 files changed, 65 insertions(+), 38 deletions(-) diff --git a/src/action_class_concept.c b/src/action_class_concept.c index 19589c458..3350e51ad 100644 --- a/src/action_class_concept.c +++ b/src/action_class_concept.c @@ -305,3 +305,45 @@ static grib_concept_value* get_concept(grib_handle* h, grib_action_concept* self GRIB_MUTEX_UNLOCK(&mutex); return result; } + +/* Caller has to allocate space for the result. + * Example: key='typeOfLevel' whose value is 'mixedLayerDepth', + * result='typeOfFirstFixedSurface=169,typeOfSecondFixedSurface=255' + */ +int get_concept_condition_string(grib_handle* h, const char* key, char* result) +{ + int err = 0; + int length = 0; + char strVal[64]={0,}; + size_t len = sizeof(strVal); + grib_concept_value* concept_value = NULL; + grib_accessor* acc = grib_find_accessor(h, key); + if (!acc) return GRIB_NOT_FOUND; + + err = grib_get_string(h, key, strVal,&len); + if (err) return GRIB_INTERNAL_ERROR; + + concept_value = action_concept_get_concept(acc); + while (concept_value) { + int err = 0; + long lres = 0; + grib_concept_condition* concept_condition = concept_value->conditions; + + if (strcmp(strVal, concept_value->name)==0) { + while (concept_condition) { + grib_expression* expression = concept_condition->expression; + Assert(expression); + err = grib_expression_evaluate_long(h,expression,&lres); + if (err) return err; + length += sprintf(result+length, "%s%s=%ld", + (length==0?"":","),concept_condition->name, lres); + + concept_condition = concept_condition->next; + } + } + + concept_value = concept_value->next; + } + if (length == 0) return GRIB_CONCEPT_NO_MATCH; + return GRIB_SUCCESS; +} diff --git a/src/grib_api_prototypes.h b/src/grib_api_prototypes.h index d593ef3dc..6412ec4c8 100644 --- a/src/grib_api_prototypes.h +++ b/src/grib_api_prototypes.h @@ -67,6 +67,7 @@ grib_action *grib_action_create_when(grib_context *context, grib_expression *exp grib_concept_value *action_concept_get_concept(grib_accessor *a); int action_concept_get_nofail(grib_accessor *a); grib_action *grib_action_create_concept(grib_context *context, const char *name, grib_concept_value *concept, const char *basename, const char *name_space, const char *defaultkey, const char *masterDir, const char *localDir, const char *ecmfDir, int flags, int nofail); +int get_concept_condition_string(grib_handle* h, const char* key, char* result); /* action_class_hash_array.c */ grib_action *grib_action_create_hash_array(grib_context *context, const char *name, grib_hash_array_value *hash_array, const char *basename, const char *name_space, const char *defaultkey, const char *masterDir, const char *localDir, const char *ecmfDir, int flags, int nofail); diff --git a/src/grib_util.c b/src/grib_util.c index bed06e5e5..cee76e208 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2108,44 +2108,6 @@ size_t sum_of_pl_array(const long* pl, size_t plsize) return count; } -static int get_concept_condition_string(grib_handle* h, const char* key, char* result) -{ - int err = 0; - int length = 0; - char strVal[32]={0,}; - size_t len = sizeof(strVal); - grib_concept_value* concept_value = NULL; - grib_accessor* acc = grib_find_accessor(h, key); - if (!acc) return GRIB_NOT_FOUND; - - err = grib_get_string(h, key, strVal,&len); - if (err) return GRIB_INTERNAL_ERROR; - - concept_value = action_concept_get_concept(acc); - while (concept_value) { - int err = 0; - long lres = 0; - grib_concept_condition* concept_condition = concept_value->conditions; - - if (strcmp(strVal, concept_value->name)==0) { - while (concept_condition) { - grib_expression* expression = concept_condition->expression; - Assert(expression); - err = grib_expression_evaluate_long(h,expression,&lres); - if (err) return err; - length += sprintf(result+length, "%s%s=%ld", - (length==0?"":","),concept_condition->name, lres); - - concept_condition = concept_condition->next; - } - } - - concept_value = concept_value->next; - } - if (length == 0) return GRIB_CONCEPT_NO_MATCH; - return GRIB_SUCCESS; -} - int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max_val) { int err = 0; diff --git a/tests/unit_tests.c b/tests/unit_tests.c index fda10940f..61ff6fdec 100644 --- a/tests/unit_tests.c +++ b/tests/unit_tests.c @@ -1425,10 +1425,32 @@ static void test_assertion_catching() assertion_caught = 0; } +static void test_concept_condition_strings() +{ + int err = 0; + char result[128] = {0,}; + grib_handle* h = grib_handle_new_from_samples(0, "GRIB2"); + + err = get_concept_condition_string(h, "typeOfLevel", result); + assert ( !err ); + assert( strcmp(result, "typeOfFirstFixedSurface=1,typeOfSecondFixedSurface=255")==0 ); + + err = get_concept_condition_string(h, "paramId", result); + assert ( !err ); + assert( strcmp(result, "discipline=0,parameterCategory=0,parameterNumber=0")==0 ); + + err = get_concept_condition_string(h, "gridType", result); + assert ( !err ); + /*printf("%s\n", result);*/ + assert( strcmp(result, "gridDefinitionTemplateNumber=0,PLPresent=0")==0 ); +} + int main(int argc, char** argv) { /*printf("Doing unit tests. ecCodes version = %ld\n", grib_get_api_version());*/ + test_concept_condition_strings(); + test_assertion_catching(); test_gaussian_latitude_640(); From 48020d09fd70e6a6329739b2b3c57e50690c7294 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Mon, 2 Dec 2019 12:36:53 +0000 Subject: [PATCH 19/23] ECC-992: get_concept_condition_string: Add optional argument for concept value --- src/action_class_concept.c | 15 +++++++++++---- src/grib_accessor_class_concept.c | 8 ++++++++ src/grib_api_prototypes.h | 2 +- src/grib_util.c | 4 ++-- tests/unit_tests.c | 8 ++++---- 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/action_class_concept.c b/src/action_class_concept.c index 3350e51ad..159b6afba 100644 --- a/src/action_class_concept.c +++ b/src/action_class_concept.c @@ -307,21 +307,27 @@ static grib_concept_value* get_concept(grib_handle* h, grib_action_concept* self } /* Caller has to allocate space for the result. + * INPUTS: h, key and value (can be NULL) + * OUTPUT: result * Example: key='typeOfLevel' whose value is 'mixedLayerDepth', * result='typeOfFirstFixedSurface=169,typeOfSecondFixedSurface=255' */ -int get_concept_condition_string(grib_handle* h, const char* key, char* result) +int get_concept_condition_string(grib_handle* h, const char* key, const char* value, char* result) { int err = 0; int length = 0; char strVal[64]={0,}; + const char* pValue = value; size_t len = sizeof(strVal); grib_concept_value* concept_value = NULL; grib_accessor* acc = grib_find_accessor(h, key); if (!acc) return GRIB_NOT_FOUND; - err = grib_get_string(h, key, strVal,&len); - if (err) return GRIB_INTERNAL_ERROR; + if (!value) { + err = grib_get_string(h, key, strVal,&len); + if (err) return GRIB_INTERNAL_ERROR; + pValue = strVal; + } concept_value = action_concept_get_concept(acc); while (concept_value) { @@ -329,10 +335,11 @@ int get_concept_condition_string(grib_handle* h, const char* key, char* result) long lres = 0; grib_concept_condition* concept_condition = concept_value->conditions; - if (strcmp(strVal, concept_value->name)==0) { + if (strcmp(pValue, concept_value->name)==0) { while (concept_condition) { grib_expression* expression = concept_condition->expression; Assert(expression); + /* TODO: Call concept_condition_expression_true to check if this condition is actually TRUE! */ err = grib_expression_evaluate_long(h,expression,&lres); if (err) return err; length += sprintf(result+length, "%s%s=%ld", diff --git a/src/grib_accessor_class_concept.c b/src/grib_accessor_class_concept.c index 40d2f2c1a..fcfb44c90 100644 --- a/src/grib_accessor_class_concept.c +++ b/src/grib_accessor_class_concept.c @@ -615,6 +615,14 @@ static int unpack_string (grib_accessor* a, char* val, size_t *len) } strcpy(val,p); *len = slen; +#if 0 + if (a->context->debug==1) { + int err = 0; + char result[1024] = {0,}; + err = get_concept_condition_string(grib_handle_of_accessor(a), a->name, val, result); + if (!err) fprintf(stderr, "ECCODES DEBUG concept name=%s, value=%s, conditions=%s\n", a->name, val, result); + } +#endif return GRIB_SUCCESS; } diff --git a/src/grib_api_prototypes.h b/src/grib_api_prototypes.h index 6412ec4c8..2a4aef5d1 100644 --- a/src/grib_api_prototypes.h +++ b/src/grib_api_prototypes.h @@ -67,7 +67,7 @@ grib_action *grib_action_create_when(grib_context *context, grib_expression *exp grib_concept_value *action_concept_get_concept(grib_accessor *a); int action_concept_get_nofail(grib_accessor *a); grib_action *grib_action_create_concept(grib_context *context, const char *name, grib_concept_value *concept, const char *basename, const char *name_space, const char *defaultkey, const char *masterDir, const char *localDir, const char *ecmfDir, int flags, int nofail); -int get_concept_condition_string(grib_handle* h, const char* key, char* result); +int get_concept_condition_string(grib_handle* h, const char* key, const char* value, char* result); /* action_class_hash_array.c */ grib_action *grib_action_create_hash_array(grib_context *context, const char *name, grib_hash_array_value *hash_array, const char *basename, const char *name_space, const char *defaultkey, const char *masterDir, const char *localDir, const char *ecmfDir, int flags, int nofail); diff --git a/src/grib_util.c b/src/grib_util.c index cee76e208..9c262132f 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2139,7 +2139,7 @@ int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max if (min_val < dmin_allowed) { char description[1024] = {0,}; - if (get_concept_condition_string(h, "param_value_min", description)==GRIB_SUCCESS) { + if (get_concept_condition_string(h, "param_value_min", NULL, description)==GRIB_SUCCESS) { fprintf(stderr, "ECCODES %s : (%s): minimum (%g) is less than the allowable limit (%g)\n", (is_error? "ERROR":"WARNING"), description, min_val, dmin_allowed); } @@ -2149,7 +2149,7 @@ int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max } if (max_val > dmax_allowed) { char description[1024] = {0,}; - if (get_concept_condition_string(h, "param_value_max", description)==GRIB_SUCCESS) { + if (get_concept_condition_string(h, "param_value_max", NULL, description)==GRIB_SUCCESS) { fprintf(stderr, "ECCODES %s : (%s): maximum (%g) is more than the allowable limit (%g)\n", (is_error? "ERROR":"WARNING"), description, max_val, dmax_allowed); } diff --git a/tests/unit_tests.c b/tests/unit_tests.c index 61ff6fdec..3795358a4 100644 --- a/tests/unit_tests.c +++ b/tests/unit_tests.c @@ -1428,18 +1428,18 @@ static void test_assertion_catching() static void test_concept_condition_strings() { int err = 0; - char result[128] = {0,}; + char result[1024] = {0,}; grib_handle* h = grib_handle_new_from_samples(0, "GRIB2"); - err = get_concept_condition_string(h, "typeOfLevel", result); + err = get_concept_condition_string(h, "typeOfLevel", NULL, result); assert ( !err ); assert( strcmp(result, "typeOfFirstFixedSurface=1,typeOfSecondFixedSurface=255")==0 ); - err = get_concept_condition_string(h, "paramId", result); + err = get_concept_condition_string(h, "paramId", NULL, result); assert ( !err ); assert( strcmp(result, "discipline=0,parameterCategory=0,parameterNumber=0")==0 ); - err = get_concept_condition_string(h, "gridType", result); + err = get_concept_condition_string(h, "gridType", NULL, result); assert ( !err ); /*printf("%s\n", result);*/ assert( strcmp(result, "gridDefinitionTemplateNumber=0,PLPresent=0")==0 ); From 0b990ac2aa50872470934d02affba8e3244ff680 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Mon, 2 Dec 2019 13:51:02 +0000 Subject: [PATCH 20/23] ECC-992: If no concept, then print paramId --- src/grib_util.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/grib_util.c b/src/grib_util.c index 9c262132f..32651a19b 100644 --- a/src/grib_util.c +++ b/src/grib_util.c @@ -2112,6 +2112,7 @@ int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max { int err = 0; long min_field_value_allowed=0, max_field_value_allowed=0; + long paramId = 0; double dmin_allowed=0, dmax_allowed=0; grib_context* ctx = h->context; int is_error = 1; @@ -2142,6 +2143,11 @@ int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max if (get_concept_condition_string(h, "param_value_min", NULL, description)==GRIB_SUCCESS) { fprintf(stderr, "ECCODES %s : (%s): minimum (%g) is less than the allowable limit (%g)\n", (is_error? "ERROR":"WARNING"), description, min_val, dmin_allowed); + } else { + if (grib_get_long(h, "paramId", ¶mId) == GRIB_SUCCESS) { + fprintf(stderr, "ECCODES %s : (paramId=%ld): minimum (%g) is less than the default allowable limit (%g)\n", + (is_error? "ERROR":"WARNING"), paramId, min_val, dmin_allowed); + } } if (is_error) { return GRIB_OUT_OF_RANGE; /* Failure */ @@ -2152,6 +2158,11 @@ int grib_util_grib_data_quality_check(grib_handle* h, double min_val, double max if (get_concept_condition_string(h, "param_value_max", NULL, description)==GRIB_SUCCESS) { fprintf(stderr, "ECCODES %s : (%s): maximum (%g) is more than the allowable limit (%g)\n", (is_error? "ERROR":"WARNING"), description, max_val, dmax_allowed); + } else { + if (grib_get_long(h, "paramId", ¶mId) == GRIB_SUCCESS) { + fprintf(stderr, "ECCODES %s : (paramId=%ld): maximum (%g) is more than the default allowable limit (%g)\n", + (is_error? "ERROR":"WARNING"), paramId, max_val, dmax_allowed); + } } if (is_error) { return GRIB_OUT_OF_RANGE; /* Failure */ From 8fe48330e7ee643d438c5518b55f9324cffc834e Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Mon, 2 Dec 2019 13:51:41 +0000 Subject: [PATCH 21/23] ECC-992: Check for truth value of concept condition --- src/action_class_concept.c | 56 ++++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/src/action_class_concept.c b/src/action_class_concept.c index 159b6afba..81a944d21 100644 --- a/src/action_class_concept.c +++ b/src/action_class_concept.c @@ -306,6 +306,50 @@ static grib_concept_value* get_concept(grib_handle* h, grib_action_concept* self return result; } +static int concept_condition_expression_true(grib_handle* h,grib_concept_condition* c) { + long lval; + long lres=0; + int ok = 0; + int err=0; + const int type = grib_expression_native_type(h,c->expression); + + switch(type) + { + case GRIB_TYPE_LONG: + grib_expression_evaluate_long(h,c->expression,&lres); + ok = (grib_get_long(h,c->name,&lval) == GRIB_SUCCESS) && + (lval == lres); + break; + + case GRIB_TYPE_DOUBLE: { + double dval; + double dres=0.0; + grib_expression_evaluate_double(h,c->expression,&dres); + ok = (grib_get_double(h,c->name,&dval) == GRIB_SUCCESS) && + (dval == dres); + break; + } + + case GRIB_TYPE_STRING: { + const char *cval; + char buf[80]; + char tmp[80]; + size_t len = sizeof(buf); + size_t size=sizeof(tmp); + + ok = (grib_get_string(h,c->name,buf,&len) == GRIB_SUCCESS) && + ((cval = grib_expression_evaluate_string(h,c->expression,tmp,&size,&err)) != NULL) && + (err==0) && (strcmp(buf,cval) == 0); + break; + } + + default: + /* TODO: */ + break; + } + return ok; +} + /* Caller has to allocate space for the result. * INPUTS: h, key and value (can be NULL) * OUTPUT: result @@ -339,12 +383,12 @@ int get_concept_condition_string(grib_handle* h, const char* key, const char* va while (concept_condition) { grib_expression* expression = concept_condition->expression; Assert(expression); - /* TODO: Call concept_condition_expression_true to check if this condition is actually TRUE! */ - err = grib_expression_evaluate_long(h,expression,&lres); - if (err) return err; - length += sprintf(result+length, "%s%s=%ld", - (length==0?"":","),concept_condition->name, lres); - + if (concept_condition_expression_true(h, concept_condition)) { + err = grib_expression_evaluate_long(h,expression,&lres); + if (err) return err; + length += sprintf(result+length, "%s%s=%ld", + (length==0?"":","),concept_condition->name, lres); + } concept_condition = concept_condition->next; } } From 4308492cb07d6d1bacf67bf7650cc6b22cebe9e2 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Mon, 2 Dec 2019 14:55:07 +0000 Subject: [PATCH 22/23] Fix memory leak --- src/grib_accessor_class_scale_values.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/grib_accessor_class_scale_values.c b/src/grib_accessor_class_scale_values.c index 4375a5389..617549f9b 100644 --- a/src/grib_accessor_class_scale_values.c +++ b/src/grib_accessor_class_scale_values.c @@ -191,8 +191,10 @@ static int pack_double(grib_accessor* a, const double* val, size_t *len) } } - if((ret = grib_set_double_array_internal(h, self->values,values,size)) != GRIB_SUCCESS) return ret; - + if((ret = grib_set_double_array_internal(h, self->values,values,size)) != GRIB_SUCCESS) { + grib_context_free(c,values); + return ret; + } grib_context_free(c,values); return GRIB_SUCCESS; From 397ec353a602bd7c7d3230dfe089307ec43018d1 Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Mon, 2 Dec 2019 14:55:29 +0000 Subject: [PATCH 23/23] ECC-992: Fix case when concept condition has a string value --- src/action_class_concept.c | 17 +++++++++-------- tests/unit_tests.c | 5 +++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/action_class_concept.c b/src/action_class_concept.c index 81a944d21..71e3ab2fc 100644 --- a/src/action_class_concept.c +++ b/src/action_class_concept.c @@ -306,7 +306,8 @@ static grib_concept_value* get_concept(grib_handle* h, grib_action_concept* self return result; } -static int concept_condition_expression_true(grib_handle* h,grib_concept_condition* c) { +static int concept_condition_expression_true(grib_handle* h, grib_concept_condition* c, char* exprVal) +{ long lval; long lres=0; int ok = 0; @@ -319,6 +320,7 @@ static int concept_condition_expression_true(grib_handle* h,grib_concept_conditi grib_expression_evaluate_long(h,c->expression,&lres); ok = (grib_get_long(h,c->name,&lval) == GRIB_SUCCESS) && (lval == lres); + if (ok) sprintf(exprVal, "%ld", lres); break; case GRIB_TYPE_DOUBLE: { @@ -327,6 +329,7 @@ static int concept_condition_expression_true(grib_handle* h,grib_concept_conditi grib_expression_evaluate_double(h,c->expression,&dres); ok = (grib_get_double(h,c->name,&dval) == GRIB_SUCCESS) && (dval == dres); + if (ok) sprintf(exprVal, "%g", dres); break; } @@ -340,6 +343,7 @@ static int concept_condition_expression_true(grib_handle* h,grib_concept_conditi ok = (grib_get_string(h,c->name,buf,&len) == GRIB_SUCCESS) && ((cval = grib_expression_evaluate_string(h,c->expression,tmp,&size,&err)) != NULL) && (err==0) && (strcmp(buf,cval) == 0); + if (ok) sprintf(exprVal, "%s", cval); break; } @@ -361,6 +365,7 @@ int get_concept_condition_string(grib_handle* h, const char* key, const char* va int err = 0; int length = 0; char strVal[64]={0,}; + char exprVal[256]={0,}; const char* pValue = value; size_t len = sizeof(strVal); grib_concept_value* concept_value = NULL; @@ -375,19 +380,15 @@ int get_concept_condition_string(grib_handle* h, const char* key, const char* va concept_value = action_concept_get_concept(acc); while (concept_value) { - int err = 0; - long lres = 0; grib_concept_condition* concept_condition = concept_value->conditions; if (strcmp(pValue, concept_value->name)==0) { while (concept_condition) { grib_expression* expression = concept_condition->expression; Assert(expression); - if (concept_condition_expression_true(h, concept_condition)) { - err = grib_expression_evaluate_long(h,expression,&lres); - if (err) return err; - length += sprintf(result+length, "%s%s=%ld", - (length==0?"":","),concept_condition->name, lres); + if (concept_condition_expression_true(h, concept_condition, exprVal)) { + length += sprintf(result+length, "%s%s=%s", + (length==0?"":","),concept_condition->name, exprVal); } concept_condition = concept_condition->next; } diff --git a/tests/unit_tests.c b/tests/unit_tests.c index 3795358a4..45bb6c2f9 100644 --- a/tests/unit_tests.c +++ b/tests/unit_tests.c @@ -1443,6 +1443,11 @@ static void test_concept_condition_strings() assert ( !err ); /*printf("%s\n", result);*/ assert( strcmp(result, "gridDefinitionTemplateNumber=0,PLPresent=0")==0 ); + + err = get_concept_condition_string(h, "stepType", NULL, result); + assert ( !err ); + assert( strcmp(result, "selectStepTemplateInstant=1,stepTypeInternal=instant")==0 ); + } int main(int argc, char** argv)