Climatology function breaks with `season == "ANN"` and drivers silently ignores errors with fallback behavior

[tomvothecoder]

What happened?

I was writing a new climo() function in #677 and found a bug in the existing climo() function. The existing climo() function does not have a conditional statement for season == "ANN", causing this function to break if this case is true.

e3sm_diags/e3sm_diags/driver/utils/climo.py

Lines 47 to 66 in 4a82b0e

	# Compute climatology
	if season == "ANNUALCYCLE":
	cycle = [
	"01",
	"02",
	"03",
	"04",
	"05",
	"06",
	"07",
	"08",
	"09",
	"10",
	"11",
	"12",
	]
	elif season == "SEASONALCYCLE":
	cycle = ["DJF", "MAM", "JJA", "SON"]
	else:
	cycle = [season]

Downstream in the lat_lon_driver.run_diag() function, there is a section of code to get the land_frac and ocean_frac.

e3sm_diags/e3sm_diags/driver/lat_lon_driver.py

Lines 132 to 142 in 4a82b0e

	# Get land/ocean fraction for masking.
	try:
	land_frac = test_data.get_climo_variable("LANDFRAC", season)
	ocean_frac = test_data.get_climo_variable("OCNFRAC", season)
	except Exception:
	mask_path = os.path.join(
	e3sm_diags.INSTALL_PATH, "acme_ne30_ocean_land_mask.nc"
	)
	with cdms2.open(mask_path) as f:
	land_frac = f("LANDFRAC")
	ocean_frac = f("OCNFRAC")

• If we run with "ANN", this section of code silently ignores this error due to except Exception (which captures all possible exceptions).
• As a result, the exception will open the default mask file (acme_ne30_ocean_land_mask.nc) in all cases even if the masking variables exist in the test data and the climatology can be calculated for them.

What did you expect to happen?

1. climo() should support "ANN"
2. lat_lon_driver.run_diag() should only capture specific exceptions (e.g,. RunTimeError) and produce a logger warning if captured
3. Check all other drivers for similar cases as 2. and fix them

Minimal Complete Verifiable Example

Relevant log output

Anything else we need to know?

In PR #677 (work-in-progress), I wrote a new climo() function that resolves this issue:
https://github.com/E3SM-Project/e3sm_diags/pull/677/files#diff-682f63237e8473278ef0ba3c452382fd62c09bb5cd23a435c8397dfd1ae98487

Environment

Latest stable release and main branch

[tomvothecoder]

@chengzhuzhang @golaz @forsyth2 After another review, I found that there is no bug with climo() when testing on the main branch.

The climo() conditional does capture if season = "ANN" using the last else statement. The season_idx dictionary has "ANN" defined.

e3sm_diags/e3sm_diags/driver/utils/climo.py

Lines 47 to 66 in 4a82b0e

	# Compute climatology
	if season == "ANNUALCYCLE":
	cycle = [
	"01",
	"02",
	"03",
	"04",
	"05",
	"06",
	"07",
	"08",
	"09",
	"10",
	"11",
	"12",
	]
	elif season == "SEASONALCYCLE":
	cycle = ["DJF", "MAM", "JJA", "SON"]
	else:
	cycle = [season]

e3sm_diags/e3sm_diags/driver/utils/climo.py

Lines 11 to 29 in 4a82b0e

	season_idx = {
	"01": [1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
	"02": [0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
	"03": [0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0],
	"04": [0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0],
	"05": [0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0],
	"06": [0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0],
	"07": [0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0],
	"08": [0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0],
	"09": [0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0],
	"10": [0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0],
	"11": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0],
	"12": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1],
	"DJF": [1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1],
	"MAM": [0, 0, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0],
	"JJA": [0, 0, 0, 0, 0, 1, 1, 1, 0, 0, 0, 0],
	"SON": [0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 0],
	"ANN": [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
	}

There might actually be an issue in my branch of PR #677 that is throwing an exception which gets caught silently with the try except Exception statement. In any case, it is good practice to use a specific exception statement (E.g., except RunTimeError), which I will include in PR #677.

Closing this issue.