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.

Sign up for GitHub

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

generate ADF config file #142

Merged
merged 23 commits into from
Nov 18, 2024
Merged

generate ADF config file #142

Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fd4db19
really adding the script this time (??)
brianpm Oct 14, 2024
97565d0
added newline at end of file per linting suggestion
brianpm Oct 14, 2024
bd92dc1
just more linting/formatting on my end
brianpm Oct 14, 2024
49a0f99
add more details for ADF config
brianpm Oct 23, 2024
c9759dc
Merge branch 'NCAR:main' into gen_adf_config
brianpm Oct 23, 2024
13c0620
fix typo for sname
brianpm Oct 23, 2024
eb25a31
ran pre-commit
TeaganKing Nov 4, 2024
9fd4ca9
minor change & note sname portion
TeaganKing Nov 4, 2024
9ed14fd
linting suggestion from after commit
TeaganKing Nov 4, 2024
9ba9d6d
update path to adf
TeaganKing Nov 4, 2024
ab8d539
include case name and not just CESM output dir
TeaganKing Nov 4, 2024
db5a6f9
included hist_str for top level rather than case-specific
TeaganKing Nov 4, 2024
db47343
updates to config file generation to include full path to ADF output …
TeaganKing Nov 6, 2024
35a9ebb
include _build/html in path
TeaganKing Nov 8, 2024
8f79704
include option for overwriting vars in adf config file
TeaganKing Nov 13, 2024
0388b54
initial response to review comments
TeaganKing Nov 15, 2024
d8a563f
additional review updates
TeaganKing Nov 15, 2024
634c2a2
Improve how we get diag_var_list
mnlevy1981 Nov 15, 2024
605a59c
Pass CI tests
mnlevy1981 Nov 16, 2024
a2308d7
update hist_str line
TeaganKing Nov 18, 2024
677a668
remove .get from base_case_name
TeaganKing Nov 18, 2024
101e1ac
update if statements for base case name
TeaganKing Nov 18, 2024
750e005
remove if statement cam_case_name
TeaganKing Nov 18, 2024
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
50 changes: 23 additions & 27 deletions helper_scripts/generate_adf_config_file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,9 @@ def generate_adf_config(cesm_root, cupid_example, adf_file, out_file):

# Set case names for ADF config
a_dict["diag_cam_climo"]["cam_case_name"] = test_case_name
if base_case_name:
a_dict["diag_cam_baseline_climo"]["cam_case_name"] = base_case_name
a_dict["diag_cam_baseline_climo"]["cam_case_name"] = base_case_name
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it okay for this value to be None? Do we need something like

if base_case_name:
  a_dict["diag_cam_baseline_climo"]["cam_case_name"] = base_case_name
elif "cam_case_name" in "cam_case_name":
  del a_dict["diag_cam_baseline_climo"]["cam_case_name"]

Copy link
Collaborator

Choose a reason for hiding this comment

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

Following up because I think the github formatting made it hard to see the context of my comment. This is specifically referring to a_dict["diag_cam_baseline_climo"]["cam_case_name"] = base_case_name, since base_case_name = c_dict["global_params"].get("base_case_name") -- what is the expected behavior if base_case_name is not in global_params?

Copy link
Collaborator

Choose a reason for hiding this comment

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

In the ADF config file, compare_obs determines whether the model run is compared to observations or another model run. If this variable is false in the ADF config file, then diag_cam_baseline_climo is used and is a required config file section. If not present, ADF fails.

I'm curious if we would want to support comparison to obs in addition to model-model comparison-- @brianpm and @justin-richling do you have thoughts on this?

With all that said, I think that we could just use base_case_name = c_dict["global_params"]["base_case_name"] instead of the .get() since it is an expected value for model-model comparisons. I'll update this now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should allow for the model vs obs comparisons. In adf_info.py there is the variable data_name that gets set for the baseline case name if model vs model, but data_name gets set to "Obs" if its a model vs obs case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit hesitant to include this feature within this PR if we want to get this in before Wednesday. I think it will require an additional default configuration file from ADF as well as a number of additional parameters in the cupid configuration file (including compare_obs) as well as parameters that are added to the script to generate the ADF config file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

See issue #150


elif "cam_case_name" in "cam_case_name":
if "cam_case_name" in "cam_case_name":
del a_dict["diag_cam_baseline_climo"]["cam_case_name"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we want to remove these two lines. The if statement isn't posed very well (it should probably be if "cam_case_name" in a_dict["diag_cam_baseline_climo"]:), but it was originally meant to catch the case where a_dict["diag_cam_baseline_climo"]["cam_case_name"] was read in from the template file and we wanted to remove it because base_case_name wasn't defined. Now we are always setting a_dict["diag_cam_baseline_climo"]["cam_case_name"] = base_case_name so we don't want to remove the dictionary entry.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done!


# TEST CASE HISTORY FILE PATH
Expand Down Expand Up @@ -122,31 +121,28 @@ def generate_adf_config(cesm_root, cupid_example, adf_file, out_file):
a_dict["diag_cam_climo"]["end_year"] = end_date

# Set values for BASELINE
if base_case_name is not None:
base_case_cupid_ts_index = (
ts_case_names.index(base_case_name)
if base_case_name in ts_case_names
else None
)
base_case_cupid_ts_index = (
ts_case_names.index(base_case_name) if base_case_name in ts_case_names else None
)

base_case_output_dir = c_dict["global_params"].get(
"base_case_output_dir",
DOUT + "/" + base_case_name,
)
base_start_date = get_date_from_ts(
c_ts["atm"],
"start_years",
base_case_cupid_ts_index,
)
base_end_date = get_date_from_ts(
c_ts["atm"],
"end_years",
base_case_cupid_ts_index,
)
if base_start_date is None:
base_start_date = start_date
if base_end_date is None:
base_end_date = end_date
base_case_output_dir = c_dict["global_params"].get(
"base_case_output_dir",
DOUT + "/" + base_case_name,
)
base_start_date = get_date_from_ts(
c_ts["atm"],
"start_years",
base_case_cupid_ts_index,
)
base_end_date = get_date_from_ts(
c_ts["atm"],
"end_years",
base_case_cupid_ts_index,
)
if base_start_date is None:
base_start_date = start_date
if base_end_date is None:
base_end_date = end_date

a_dict["diag_cam_baseline_climo"]["cam_hist_loc"] = os.path.join(
base_case_output_dir,
Expand Down
Loading