Conversation
forsyth2
left a comment
There was a problem hiding this comment.
@chengzhuzhang Ok, I think these changes look good. The unit tests and integration test cfgs will need to be updated though. I can add a commit for those changes if you'd like. Thanks.
zppy/defaults/default.ini
Outdated
| output_format_subplot = string_list(default=list()) | ||
| # End year (i.e., the last year to use) for the reference data | ||
| # Required for "tropical_subseasonal" runs | ||
| ref_end_yr = string(default="") |
There was a problem hiding this comment.
We can probably just remove this then.
| def check_parameters_for_bash(c: Dict[str, Any]) -> None: | ||
| # Check parameters that aren't used until e3sm_diags.bash is run | ||
| check_required_parameters(c, set(["tropical_subseasonal"]), "ref_end_yr") | ||
| check_required_parameters(c, set(["qbo"]), "ref_final_yr") |
There was a problem hiding this comment.
In zppy/templates/e3sm_diags.bash, we have:
{% if run_type == "model_vs_obs" %}
# Obs
trop_param.reference_data_path = '{{ obs_ts }}'
trop_param.ref_start_yr = 2001
trop_param.ref_end_yr = 2010Those are hard coded, so we're good to delete this check here, yes
| trop_param.test_start_yr = f'{start_yr:04}' | ||
| trop_param.test_end_yr = f'{end_yr:04}' |
There was a problem hiding this comment.
I think these are always put in as 4-digit years, so this change should be ok
|
@forsyth2 thanks for a review. Could you go ahead to update the tests? Thanks. |
|
Added a 3rd commit, will use merge commit to preserve authorship of the different commits. Unit tests pass. |
Summary
The problem might be introduced from abad674. This PR is to revert the change regarding to
ref_end_yr:ref_final_yras other variability sets.Issue resolution:
Select one: This pull request is...
Please fill out either the "Small Change" or "Big Change" section (the latter includes the numbered subsections), and delete the other.
Small Change
Big Change
1. Does this do what we want it to do?
Required:
If applicable:
2. Are the implementation details accurate & efficient?
Required:
If applicable:
zppy/conda, not just animportstatement.3. Is this well documented?
Required:
4. Is this code clean?
Required:
If applicable: