[Bug]: Fix default diagnostic parameters not being copied correctly to parent CoreParameter resulting in AttributeError

[tomvothecoder]

What happened?

I ran into this issue when trying to run ex1.py with default diagnostics on e3sm_diags >=2.10.0 and main.

In #747, some code was refactored in _get_parameters_with_api_and_cfg() to only get cfg_params once.

e3sm_diags/e3sm_diags/run.py

Lines 168 to 172 in c8ffba9

	if self.is_cfg_file_arg_set:
	cfg_params = self._get_custom_params_from_cfg_file()
	else:
	run_type = parameters[0].run_type
	cfg_params = self._get_default_params_from_cfg_file(run_type)

Then a deepcopy() is performed on cfg_params over every iteration of for set_name in self.sets_to_run.

e3sm_diags/e3sm_diags/run.py

Lines 189 to 199 in c8ffba9

	# # FIXME: Make a deep copy of cfg_params because there is some
	# buggy code in this method that changes parameter attributes in
	# place, which affects downstream operations. The original
	# cfg_params needs to be perserved for each iteration of this
	# for loop.
	params = self.parser.get_parameters(
	orig_parameters=param,
	other_parameters=copy.deepcopy(cfg_params),
	cmd_default_vars=False,
	argparse_vals_only=False,
	)

I wanted to improve performance by only instantiating cfg_params once because it should not change. However, this change seemed to cause an issue where the default diagnostic parameters are not being copied to the parent CoreParameter object, resulting in AttributeError (look at log).

What did you expect to happen? Are there are possible answers you came across?

The fix is to move the conditional that sets cfg_params back into the for loop as it was prior to #747. Example below:

        for set_name in self.sets_to_run:
            if self.is_cfg_file_arg_set:
                cfg_params = self._get_custom_params_from_cfg_file()
            else:
                run_type = parameters[0].run_type
                cfg_params = self._get_default_params_from_cfg_file(run_type)

            param = self._get_instance_of_param_class(
                SET_TO_PARAMETERS[set_name], parameters
            )

            # Since each parameter will have lots of default values, we want to
            # remove them. Otherwise when calling get_parameters(), these
            # default values will take precedence over values defined in
            # other_params.
            self._remove_attrs_with_default_values(param)
            param.sets = [set_name]

            # # FIXME: Make a deep copy of cfg_params because there is some
            # buggy code in this method that changes parameter attributes in
            # place, which affects downstream operations. The original
            # cfg_params needs to be perserved for each iteration of this
            # for loop.
            params = self.parser.get_parameters(
                orig_parameters=param,
                other_parameters=cfg_params,
                cmd_default_vars=False,
                argparse_vals_only=False,
            )

Minimal Complete Verifiable Example (MVCE)

# %%
import os

from e3sm_diags.parameter.core_parameter import CoreParameter
from e3sm_diags.run import runner

param = CoreParameter()

# Location of the data.
param.test_data_path = "/global/cfs/cdirs/e3sm/e3sm_diags/test_model_data_for_acme_diags/time-series/E3SM_v1"
param.reference_data_path = "/global/cfs/cdirs/e3sm/e3sm_diags/test_model_data_for_acme_diags/time-series/E3SM_v1"

# Set this parameter to True.
# By default, e3sm_diags expects the test data to be climo data.
param.test_timeseries_input = True
# Years to slice the test data, base this off the years in the filenames.
param.test_start_yr = "2011"
param.test_end_yr = "2013"

# Set this parameter to True.
# By default, e3sm_diags expects the ref data to be climo data.
param.ref_timeseries_input = True
# Years to slice the ref data, base this off the years in the filenames
param.ref_start_yr = "1850"
param.ref_end_yr = "1852"

# When running with time-series data, you don't need to specify the name of the data.
# But you should, otherwise nothing is displayed when the test/ref name is needed.
param.short_test_name = "historical_H1"
param.short_ref_name = "historical_H1"

# This parameter modifies the software to accommodate model vs model runs.
# The default setting for run_type is 'model_vs_obs'.
param.run_type = "model_vs_model"
# Name of the folder where the results are stored.
# Change `prefix` to use your directory.
prefix = "/global/cfs/cdirs/e3sm/www/vo13/examples"
param.results_dir = os.path.join(prefix, "755_main_test")

# Below are more optional arguments.

# What plotsets to run the diags on.
# If not defined, then all available sets are used.
param.sets = ["lat_lon"]
# What seasons to run the diags on.
# If not defined, diags are run on ['ANN', 'DJF', 'MAM', 'JJA', 'SON'].
param.seasons = ["ANN"]
# Title of the difference plots.
param.diff_title = "Model (2011-2013) - Model (1850-1852)"

# For running with multiprocessing.
param.multiprocessing = True
# param.num_workers = 24

# %%
runner.run_diags([param])

# %%run

Relevant log output

2024-01-08 12:58:40,537 [INFO]: e3sm_diags_driver.py(_save_env_yml:58) >> Saved environment yml file to: /global/cfs/cdirs/e3sm/www/vo13/examples/755_main_test/prov/environment.yml
2024-01-08 12:58:40,539 [INFO]: e3sm_diags_driver.py(_save_parameter_files:69) >> Saved command used to: /global/cfs/cdirs/e3sm/www/vo13/examples/755_main_test/prov/cmd_used.txt
2024-01-08 12:58:40,541 [INFO]: e3sm_diags_driver.py(_save_python_script:133) >> Saved Python script to: /global/cfs/cdirs/e3sm/www/vo13/examples/755_main_test/prov/ipykernel_launcher.py
2024-01-08 12:58:40,722 [ERROR]: core_parameter.py(_run_diag:267) >> Error in e3sm_diags.driver.tc_analysis_driver
Traceback (most recent call last):
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/parameter/core_parameter.py", line 264, in _run_diag
    single_result = module.run_diag(self)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/driver/tc_analysis_driver.py", line 77, in run_diag
    test_cyclones_hist = cdms2.open(test_cyclones_file)(
  File "/global/homes/v/vo13/mambaforge/envs/e3sm_diags_dev_main/lib/python3.10/site-packages/cdms2/dataset.py", line 514, in openDataset
    raise FileNotFoundError(path)
FileNotFoundError: /global/cfs/cdirs/e3sm/e3sm_diags/test_model_data_for_acme_diags/time-series/E3SM_v1/cyclones_hist__2011_2013.nc
2024-01-08 12:58:40,744 [ERROR]: core_parameter.py(_run_diag:267) >> Error in e3sm_diags.driver.arm_diags_driver
Traceback (most recent call last):
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/parameter/core_parameter.py", line 264, in _run_diag
    single_result = module.run_diag(self)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/driver/arm_diags_driver.py", line 569, in run_diag
    if parameter.diags_set == "annual_cycle":
AttributeError: 'CoreParameter' object has no attribute 'diags_set'
2024-01-08 12:58:41,343 [ERROR]: core_parameter.py(_run_diag:267) >> Error in e3sm_diags.driver.aerosol_budget_driver
Traceback (most recent call last):
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/parameter/core_parameter.py", line 264, in _run_diag
    single_result = module.run_diag(self)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/driver/aerosol_budget_driver.py", line 121, in run_diag
    variables = parameter.variables[0].split(", ")
IndexError: list index out of range
2024-01-08 12:58:41,564 [ERROR]: core_parameter.py(_run_diag:267) >> Error in e3sm_diags.driver.enso_diags_driver
Traceback (most recent call last):
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/parameter/core_parameter.py", line 264, in _run_diag
    single_result = module.run_diag(self)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/driver/enso_diags_driver.py", line 446, in run_diag
    if parameter.plot_type == "map":
AttributeError: 'CoreParameter' object has no attribute 'plot_type'
2024-01-08 12:58:41,752 [ERROR]: core_parameter.py(_run_diag:267) >> Error in e3sm_diags.driver.area_mean_time_series_driver
Traceback (most recent call last):
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/parameter/core_parameter.py", line 264, in _run_diag
    single_result = module.run_diag(self)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/driver/area_mean_time_series_driver.py", line 39, in run_diag
    ref_names = parameter.ref_names
AttributeError: 'CoreParameter' object has no attribute 'ref_names'. Did you mean: 'ref_name'?
2024-01-08 12:58:42,224 [ERROR]: core_parameter.py(_run_diag:267) >> Error in e3sm_diags.driver.streamflow_driver
Traceback (most recent call last):
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/parameter/core_parameter.py", line 264, in _run_diag
    single_result = module.run_diag(self)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/driver/streamflow_driver.py", line 102, in run_diag
    if parameter.gauges_path is None:
AttributeError: 'CoreParameter' object has no attribute 'gauges_path'
2024-01-08 12:58:42,569 [ERROR]: core_parameter.py(_run_diag:267) >> Error in e3sm_diags.driver.aerosol_aeronet_driver
Traceback (most recent call last):
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/parameter/core_parameter.py", line 264, in _run_diag
    single_result = module.run_diag(self)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/driver/aerosol_aeronet_driver.py", line 59, in run_diag
    ref = ref_data.get_climo_variable(var, season)
UnboundLocalError: local variable 'var' referenced before assignment
2024-01-08 12:58:42,948 [INFO]: main.py(create_viewer:130) >> zonal_mean_xy /global/cfs/cdirs/e3sm/www/vo13/examples/755_main_test/viewer
2024-01-08 12:58:42,950 [ERROR]: run.py(run_diags:91) >> Error traceback:
Traceback (most recent call last):
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/run.py", line 89, in run_diags
    params_results = main(params)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/e3sm_diags_driver.py", line 391, in main
    index_path = create_viewer(path, parameters_results)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/viewer/main.py", line 132, in create_viewer
    result = viewer_function(root_dir, parameters)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/viewer/default_viewer.py", line 167, in create_viewer
    _add_information_to_viewer(viewer, ROW_INFO, set_name, save_netcdf, ext)
  File "/global/u2/v/vo13/E3SM-Project/e3sm_diags_main/e3sm_diags/viewer/default_viewer.py", line 302, in _add_information_to_viewer
    for group in row_info[set_name]:
KeyError: 'zonal_mean_xy'
2024-01-08 12:58:42,954 [INFO]: logger.py(move_log_to_prov_dir:106) >> Log file saved in /global/cfs/cdirs/e3sm/www/vo13/examples/755_main_test/prov/e3sm_diags_run.log

Anything else we need to know?

No response

Environment

e3sm_diags >=2.10.0 and main