Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to diags handling #633

Merged
merged 1 commit into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ walltime = "5:00:00"

[[ atm_monthly_180x360_aave ]]
climo_subsection = "atm_monthly_180x360_aave"
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere",
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","aerosol_aeronet","aerosol_budget",
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ walltime = "5:00:00"
# Use _1 as reference
reference_data_path = "/lcrc/group/e3sm/ac.forsyth2/zppy_min_case_e3sm_diags_depend_on_climo_mvm_1_output/unique_id/v3.LR.historical_0051/post/atm/180x360_aave/clim"
run_type = "model_vs_model"
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere",
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","aerosol_aeronet","aerosol_budget",
short_ref_name = "v3.LR.historical_0051"
swap_test_ref = False
tag = "model_vs_model"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ short_name = "v3.LR.historical_0051"
walltime = "5:00:00"

[[ lnd_monthly_mvm_lnd ]]
climo_subsection = "land_monthly_climo"
climo_land_subsection = "land_monthly_climo"
diff_title = "Difference"
ref_final_yr = 1988
ref_name = "v3.LR.historical_0051"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@ ref_final_yr = 1981
ref_start_yr = 1980
ref_years = "1980-1981",
# Include all sets
# min_case_e3sm_diags_depend_on_climo: "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere",
# min_case_e3sm_diags_depend_on_climo: "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","aerosol_aeronet","aerosol_budget",
# min_case_e3sm_diags_depend_on_ts: "enso_diags","qbo",
# min_case_e3sm_diags_diurnal_cycle: "diurnal_cycle",
# min_case_e3sm_diags_streamflow: "streamflow",
# min_case_e3sm_diags_tc_analysis: "tc_analysis",
# min_case_e3sm_diags_tropical_subseasonal: "tropical_subseasonal",
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","enso_diags","qbo","diurnal_cycle","streamflow","tc_analysis","tropical_subseasonal",
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","enso_diags","qbo","diurnal_cycle","streamflow","tc_analysis","tropical_subseasonal","aerosol_aeronet","aerosol_budget",
short_name = "v2.LR.historical_0201"
ts_num_years = 2
walltime = "5:00:00"
Expand Down Expand Up @@ -139,7 +139,7 @@ years = "1982:1984:2",

[[ lnd_monthly_mvm_lnd ]]
# Test model-vs-model using the same files as the reference
climo_subsection = "land_monthly_climo"
climo_land_subsection = "land_monthly_climo"
diff_title = "Difference"
partition = "compute"
qos = "regular"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,15 @@ ref_final_yr = 1986
ref_start_yr = 1985
ref_years = "1985-1986",
# Include all sets
# min_case_e3sm_diags_depend_on_climo: "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere",
# min_case_e3sm_diags_depend_on_climo: "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","aerosol_aeronet","aerosol_budget",
# min_case_e3sm_diags_depend_on_ts: "enso_diags","qbo",
# min_case_e3sm_diags_diurnal_cycle: "diurnal_cycle",
# min_case_e3sm_diags_streamflow: "streamflow",
# min_case_e3sm_diags_tc_analysis: "tc_analysis",
# min_case_e3sm_diags_tropical_subseasonal: "tropical_subseasonal",
# TODO: Add "tc_analysis" back in after empty dat is resolved.
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","enso_diags","qbo","diurnal_cycle","streamflow","tropical_subseasonal",
# TODO: Add "aerosol_budget" back in once that's working for v3.
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","enso_diags","qbo","diurnal_cycle","streamflow","tropical_subseasonal","aerosol_aeronet",
short_name = "v3.LR.historical_0051"
ts_daily_subsection = "atm_daily_180x360_aave"
ts_num_years = 2
Expand Down Expand Up @@ -164,7 +165,7 @@ tc_obs = "/lcrc/group/e3sm/diagnostics/observations/Atm/tc-analysis/"

[[ lnd_monthly_mvm_lnd ]]
# Test model-vs-model using the same files as the reference
climo_subsection = "land_monthly_climo"
climo_land_subsection = "land_monthly_climo"
diff_title = "Difference"
partition = "compute"
qos = "regular"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ walltime = "#expand diags_walltime#"

[[ atm_monthly_180x360_aave ]]
climo_subsection = "atm_monthly_180x360_aave"
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere",
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","aerosol_aeronet","aerosol_budget",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key Change 1: adding in the aerosol sets to tests.

Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ walltime = "#expand diags_walltime#"
# Use _1 as reference
reference_data_path = "#expand user_output#zppy_min_case_e3sm_diags_depend_on_climo_mvm_1_output/#expand unique_id#/#expand case_name#/post/atm/180x360_aave/clim"
run_type = "model_vs_model"
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere",
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","aerosol_aeronet","aerosol_budget",
short_ref_name = "#expand case_name#"
swap_test_ref = False
tag = "model_vs_model"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ short_name = "#expand case_name#"
walltime = "#expand diags_walltime#"

[[ lnd_monthly_mvm_lnd ]]
climo_subsection = "land_monthly_climo"
climo_land_subsection = "land_monthly_climo"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key Change 2: specifying a climo subsection specific to land

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@forsyth2 I have a clarification question regarding to backward compatibility. What would be the consequence if in an old zppy config file, a user uses climo_subsection = "land_monthly_climo"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so that wouldn't have actually had any effect. And therefore there is nothing to be backwards compatible with.

This block is new in this PR:

    if "lat_lon_land" in c["sets"]:
        check_parameter_defined(c, "climo_land_subsection")
        dependencies.append(
            os.path.join(
                script_dir, f"climo_{c['climo_land_subsection']}{status_suffix}"
            )
        )

Notice, lat_lon_land wasn't included in:

def add_climo_dependencies(
    c: Dict[str, Any], dependencies: List[str], script_dir: str
) -> None:
    depend_on_climo: Set[str] = set(
        [
            "lat_lon",
            "zonal_mean_xy",
            "zonal_mean_2d",
            "polar",
            "cosp_histogram",
            "meridional_mean_2d",
            "annual_cycle_zonal_mean",
            "zonal_mean_2d_stratosphere",
            "aerosol_aeronet",
            "aerosol_budget",
        ]
    )

So basically, including climo_subsection as a parameter in this block of the cfg was simply unused information before this PR.

It only appeared like it was useful, because the land climo task often just happens to finish before the lnd_monthly_mvm_lnd diags task runs anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying. I think this PR is good to go!

diff_title = "Difference"
ref_final_yr = 1988
ref_name = "#expand case_name#"
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/template_weekly_comprehensive_v2.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@ ref_final_yr = 1981
ref_start_yr = 1980
ref_years = "1980-1981",
# Include all sets
# min_case_e3sm_diags_depend_on_climo: "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere",
# min_case_e3sm_diags_depend_on_climo: "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","aerosol_aeronet","aerosol_budget",
# min_case_e3sm_diags_depend_on_ts: "enso_diags","qbo",
# min_case_e3sm_diags_diurnal_cycle: "diurnal_cycle",
# min_case_e3sm_diags_streamflow: "streamflow",
# min_case_e3sm_diags_tc_analysis: "tc_analysis",
# min_case_e3sm_diags_tropical_subseasonal: "tropical_subseasonal",
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","enso_diags","qbo","diurnal_cycle","streamflow","tc_analysis","tropical_subseasonal",
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","enso_diags","qbo","diurnal_cycle","streamflow","tc_analysis","tropical_subseasonal","aerosol_aeronet","aerosol_budget",
short_name = "#expand case_name_v2#"
ts_num_years = 2
walltime = "#expand diags_walltime#"
Expand Down Expand Up @@ -139,7 +139,7 @@ years = "1982:1984:2",

[[ lnd_monthly_mvm_lnd ]]
# Test model-vs-model using the same files as the reference
climo_subsection = "land_monthly_climo"
climo_land_subsection = "land_monthly_climo"
diff_title = "Difference"
partition = "#expand partition_long#"
qos = "#expand qos_long#"
Expand Down
7 changes: 4 additions & 3 deletions tests/integration/template_weekly_comprehensive_v3.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,15 @@ ref_final_yr = 1986
ref_start_yr = 1985
ref_years = "1985-1986",
# Include all sets
# min_case_e3sm_diags_depend_on_climo: "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere",
# min_case_e3sm_diags_depend_on_climo: "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","aerosol_aeronet","aerosol_budget",
# min_case_e3sm_diags_depend_on_ts: "enso_diags","qbo",
# min_case_e3sm_diags_diurnal_cycle: "diurnal_cycle",
# min_case_e3sm_diags_streamflow: "streamflow",
# min_case_e3sm_diags_tc_analysis: "tc_analysis",
# min_case_e3sm_diags_tropical_subseasonal: "tropical_subseasonal",
# TODO: Add "tc_analysis" back in after empty dat is resolved.
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","enso_diags","qbo","diurnal_cycle","streamflow","tropical_subseasonal",
# TODO: Add "aerosol_budget" back in once that's working for v3.
sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","enso_diags","qbo","diurnal_cycle","streamflow","tropical_subseasonal","aerosol_aeronet",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left "aerosol_budget" off the weekly v3 test since that won't work at the moment.

short_name = "#expand case_name#"
ts_daily_subsection = "atm_daily_180x360_aave"
ts_num_years = 2
Expand Down Expand Up @@ -164,7 +165,7 @@ tc_obs = "#expand diagnostics_base_path#/observations/Atm/tc-analysis/"

[[ lnd_monthly_mvm_lnd ]]
# Test model-vs-model using the same files as the reference
climo_subsection = "land_monthly_climo"
climo_land_subsection = "land_monthly_climo"
diff_title = "Difference"
partition = "#expand partition_long#"
qos = "#expand qos_long#"
Expand Down
26 changes: 26 additions & 0 deletions tests/test_zppy_e3sm_diags.py
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,32 @@ def test_add_climo_dependencies(self):
dependencies = []
add_climo_dependencies(c, dependencies, "script_dir")
self.assertEqual(dependencies, ["script_dir/climo_cdsub_1980-1990.status"])
c = {"sets": ["diurnal_cycle"]}
c.update(base)
dependencies = []
self.assertRaises(
ParameterNotProvidedError,
add_climo_dependencies,
c,
dependencies,
"script_dir",
)
Comment on lines +536 to +545
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test that climo_diurnal_subsection is checked for if diurnal_cycle is requested.

(In this unit test, we have "guess_path_parameters": True, "guess_section_parameters": True, so an error will be raised if this parameter is not provided. These options are off-by-default, meaning zppy will instead try to guess the value.)


c = {"sets": ["lat_lon_land"], "climo_land_subsection": "lndsub"}
c.update(base)
dependencies = []
add_climo_dependencies(c, dependencies, "script_dir")
self.assertEqual(dependencies, ["script_dir/climo_lndsub_1980-1990.status"])
c = {"sets": ["lat_lon_land"]}
c.update(base)
dependencies = []
self.assertRaises(
ParameterNotProvidedError,
add_climo_dependencies,
c,
dependencies,
"script_dir",
)
Comment on lines +547 to +561
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test that climo_land_subsection is checked for if lat_lon_land is requested.


c = {"sets": ["tc_analysis"]}
c.update(base)
Expand Down
7 changes: 7 additions & 0 deletions tests/test_zppy_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
ParameterGuessType,
ParameterNotProvidedError,
add_dependencies,
check_parameter_defined,
check_required_parameters,
define_or_guess,
define_or_guess2,
Expand Down Expand Up @@ -460,6 +461,12 @@ def test_define_or_guess2(self):
)
self.assertEqual(c["required_parameter"], "backup_option")

def test_check_parameter_defined(self):
c = {"a": 1, "b": 2, "c": ""}
check_parameter_defined(c, "a")
self.assertRaises(ParameterNotProvidedError, check_parameter_defined, c, "c")
self.assertRaises(ParameterNotProvidedError, check_parameter_defined, c, "d")

Comment on lines +464 to +469
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test the check_parameter_defined function.

def test_get_file_names(self):
bash, settings, status = get_file_names("script_dir", "prefix")
self.assertEqual(bash, "script_dir/prefix.bash")
Expand Down
21 changes: 14 additions & 7 deletions zppy/e3sm_diags.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
from zppy.bundle import handle_bundles
from zppy.utils import (
ParameterGuessType,
ParameterNotProvidedError,
add_dependencies,
check_parameter_defined,
check_required_parameters,
check_status,
define_or_guess,
Expand Down Expand Up @@ -111,12 +111,9 @@ def check_parameters_for_bash(c: Dict[str, Any]) -> None:

def check_mvm_only_parameters_for_bash(c: Dict[str, Any]) -> None:
# Check mvm-specific parameters that aren't used until e3sm_diags.bash is run.
if c["diff_title"] == "":
raise ParameterNotProvidedError("diff_title")
if c["ref_name"] == "":
raise ParameterNotProvidedError("ref_name")
if c["short_ref_name"] == "":
raise ParameterNotProvidedError("short_ref_name")
check_parameter_defined(c, "diff_title")
check_parameter_defined(c, "ref_name")
check_parameter_defined(c, "short_ref_name")

check_required_parameters(
c,
Expand Down Expand Up @@ -244,6 +241,8 @@ def add_climo_dependencies(
"meridional_mean_2d",
"annual_cycle_zonal_mean",
"zonal_mean_2d_stratosphere",
"aerosol_aeronet",
"aerosol_budget",
Comment on lines +244 to +245
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marked the aerosol sets as depending on climo.

]
)
# Check if any requested sets depend on climo:
Expand All @@ -256,11 +255,19 @@ def add_climo_dependencies(
os.path.join(script_dir, f"climo_{climo_sub}{status_suffix}"),
)
if "diurnal_cycle" in c["sets"]:
check_parameter_defined(c, "climo_diurnal_subsection")
dependencies.append(
os.path.join(
script_dir, f"climo_{c['climo_diurnal_subsection']}{status_suffix}"
)
)
if "lat_lon_land" in c["sets"]:
check_parameter_defined(c, "climo_land_subsection")
dependencies.append(
os.path.join(
script_dir, f"climo_{c['climo_land_subsection']}{status_suffix}"
)
)
if "tc_analysis" in c["sets"]:
dependencies.append(os.path.join(script_dir, f"tc_analysis{status_suffix}"))

Expand Down
7 changes: 0 additions & 7 deletions zppy/templates/tc_analysis.bash
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,6 @@ cat ${result_dir}out.dat0* > ${result_dir}cyclones_${file_name}.txt
StitchNodes --in_fmt "lon,lat,slp,wind" --in_connect ${result_dir}connect_CSne${res}_v2.dat --range 6.0 --mintime 6 --maxgap 1 --in ${result_dir}cyclones_${file_name}.txt --out ${result_dir}cyclones_stitch_${file_name}.dat --threshold "wind,>=,17.5,6;lat,<=,40.0,6;lat,>=,-40.0,6"
rm ${result_dir}cyclones_${file_name}.txt

# If cyclones_stitch file is empty, exit
if ! [ -s ${result_dir}cyclones_stitch_${file_name}.dat ]; then
cd {{ scriptDir }}
echo 'ERROR (1)' > {{ prefix }}.status
exit 1
fi

Comment on lines -93 to -99
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key Change 3: Removed this block since this is not an error per-se, as discussed. Instead, E3SM Diags will do this check via E3SM-Project/e3sm_diags@ffbdf28

# Generate histogram of detections
HistogramNodes --in ${result_dir}cyclones_stitch_${file_name}.dat --iloncol 2 --ilatcol 3 --out ${result_dir}cyclones_hist_${file_name}.nc

Expand Down
5 changes: 5 additions & 0 deletions zppy/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,11 @@ def define_or_guess2(
raise ParameterNotProvidedError(parameter)


def check_parameter_defined(c: Dict[str, Any], relevant_parameter: str) -> None:
if (relevant_parameter not in c.keys()) or (c[relevant_parameter] == ""):
raise ParameterNotProvidedError(relevant_parameter)


def get_file_names(script_dir: str, prefix: str):
return tuple(
[
Expand Down
Loading