From d35402dda23b399e32754086e4d78c92870dd429 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Fri, 1 Nov 2024 13:07:08 -0600 Subject: [PATCH] Bugfix #2762 PCPCombine derive field info (#2764) * per #2762, change derive mode to add field info for each file instead of adding the first field info only * Per #2762, improve how the accumulation amounts are handling using relativedelta to ensure that the correct number of seconds are used for each search window when using inconsistent time intervals like months or years --- .../pcp_combine/test_pcp_combine_wrapper.py | 16 +++++----- metplus/wrappers/pcp_combine_wrapper.py | 31 +++++++++++-------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/internal/tests/pytests/wrappers/pcp_combine/test_pcp_combine_wrapper.py b/internal/tests/pytests/wrappers/pcp_combine/test_pcp_combine_wrapper.py index a0291936a..17afa3626 100644 --- a/internal/tests/pytests/wrappers/pcp_combine/test_pcp_combine_wrapper.py +++ b/internal/tests/pytests/wrappers/pcp_combine/test_pcp_combine_wrapper.py @@ -377,7 +377,7 @@ def test_pcp_combine_derive(metplus_config, get_test_data_dir, config_overrides, stat_list = 'sum,min,max,range,mean,stdev,vld_count' fcst_name = 'APCP' fcst_level = 'A03' - fcst_fmt = f'-field \'name="{fcst_name}"; level="{fcst_level}";\'' + fcst_fmt = f'\'name="{fcst_name}"; level="{fcst_level}";\'' config = metplus_config test_data_dir = get_test_data_dir() @@ -427,13 +427,13 @@ def test_pcp_combine_derive(metplus_config, get_test_data_dir, config_overrides, verbosity = f"-v {wrapper.c_dict['VERBOSITY']}" out_dir = wrapper.c_dict.get('FCST_OUTPUT_DIR') expected_cmds = [(f"{app_path} -derive {stat_list} " - f"{fcst_input_dir}/2005080700/24.tm00_G212 " - f"{fcst_input_dir}/2005080700/21.tm00_G212 " - f"{fcst_input_dir}/2005080700/18.tm00_G212 " - f"{fcst_input_dir}/2005080700/15.tm00_G212 " - f"{fcst_input_dir}/2005080700/12.tm00_G212 " - f"{fcst_input_dir}/2005080700/09.tm00_G212 " - f"{fcst_fmt} {extra_fields}" + f"{fcst_input_dir}/2005080700/24.tm00_G212 {fcst_fmt} " + f"{fcst_input_dir}/2005080700/21.tm00_G212 {fcst_fmt} " + f"{fcst_input_dir}/2005080700/18.tm00_G212 {fcst_fmt} " + f"{fcst_input_dir}/2005080700/15.tm00_G212 {fcst_fmt} " + f"{fcst_input_dir}/2005080700/12.tm00_G212 {fcst_fmt} " + f"{fcst_input_dir}/2005080700/09.tm00_G212 {fcst_fmt} " + f"{extra_fields}" f"{out_dir}/2005080700_f24_A18.nc {verbosity}"), ] diff --git a/metplus/wrappers/pcp_combine_wrapper.py b/metplus/wrappers/pcp_combine_wrapper.py index c984e5ca1..3324f519f 100755 --- a/metplus/wrappers/pcp_combine_wrapper.py +++ b/metplus/wrappers/pcp_combine_wrapper.py @@ -531,8 +531,8 @@ def setup_derive_method(self, time_info, lookback, data_src): files_found = [] for input_file in input_files: - # exclude field info and set it with -field self.args.append(input_file) + self.args.append(field_info) files_found.append((input_file, field_info)) else: @@ -540,7 +540,7 @@ def setup_derive_method(self, time_info, lookback, data_src): files_found = self.get_accumulation(time_info, lookback, data_src, - field_info_after_file=False) + field_info_after_file=True) if not files_found: self.missing_input_count += 1 msg = ( @@ -553,9 +553,6 @@ def setup_derive_method(self, time_info, lookback, data_src): self.log_error(msg) return None - # set -field name and level from first file field info - self.args.append(f'-field {files_found[0][1]}') - self._handle_input_thresh_argument(data_src) return files_found @@ -652,7 +649,8 @@ def get_accumulation(self, time_info, accum, data_src, accum_relative = get_relativedelta(accum, 'S') # using 1 hour for now smallest_input_accum = min( - [lev['amount'] for lev in self.c_dict['ACCUM_DICT_LIST']] + [ti_get_seconds_from_relativedelta(lev['amount'], search_time) + for lev in self.c_dict['ACCUM_DICT_LIST']] ) if smallest_input_accum == 9999999: smallest_input_accum = 3600 @@ -746,8 +744,9 @@ def _add_file_and_field_info_to_args(self, search_file, field_info, def _find_file_for_accum(self, accum_dict, total_accum, time_info, search_time, data_src, custom): - if (accum_dict['amount'] > total_accum and - accum_dict['template'] is None): + # get number of seconds from relativedelta accum amount using search time + accum_seconds = ti_get_seconds_from_relativedelta(accum_dict['amount'], search_time) + if accum_seconds > total_accum and accum_dict['template'] is None: return None, None, None self.c_dict['SUPPRESS_WARNINGS'] = True @@ -774,7 +773,7 @@ def _find_file_for_accum(self, accum_dict, total_accum, time_info, "than remaining accumulation.") return None, None, None else: - accum_amount = accum_dict['amount'] + accum_amount = accum_seconds search_time_info = { 'valid': search_time, @@ -809,7 +808,8 @@ def get_lowest_fcst_file(self, valid_time, data_src, custom): self.c_dict[data_src+'_MAX_FORECAST'], 'H' ) smallest_input_accum = min( - [lev['amount'] for lev in self.c_dict['ACCUM_DICT_LIST']] + [ti_get_seconds_from_relativedelta(lev['amount'], valid_time) + for lev in self.c_dict['ACCUM_DICT_LIST']] ) # if smallest input accumulation is greater than an hour, search hourly @@ -863,6 +863,9 @@ def find_input_file(self, init_time, valid_time, search_accum, data_src, custom): lead = 0 in_template = self.c_dict[data_src+'_INPUT_TEMPLATE'] + # use rel_time as offset to compute seconds from + # relativedelta search_accum for time_info level -- can be init or valid + rel_time = valid_time if ('{lead?' in in_template or ('{init?' in in_template and '{valid?' in in_template)): @@ -873,15 +876,17 @@ def find_input_file(self, init_time, valid_time, search_accum, data_src, # ti_calculate cannot currently handle both init and valid lead = (valid_time - init_time).total_seconds() input_dict = {'init': init_time, 'lead': lead} + rel_time = init_time else: if self.c_dict[f'{data_src}_CONSTANT_INIT']: input_dict = {'init': init_time} + rel_time = init_time else: input_dict = {'valid': valid_time} time_info = ti_calculate(input_dict) time_info['custom'] = custom - time_info['level'] = int(search_accum) + time_info['level'] = ti_get_seconds_from_relativedelta(search_accum, rel_time) input_path = self.find_data(time_info, data_type=data_src, return_list=True, mandatory=False) if input_path: @@ -1006,8 +1011,8 @@ def _build_input_accum_list(self, data_src, time_info): template = accum accum = '9999999S' - # convert accum amount to seconds from time string - amount = get_seconds_from_string(accum, 'H', time_info['valid']) + # convert accum amount to relativedelta from time string + amount = get_relativedelta(accum, default_unit='H') accum_dict_list.append({'amount': amount, 'name': name,