Updates in helper script for updated cupid workflow#176
Conversation
|
I'll be adding |
…criptions from xml vars
cime_config/config_tool.xml is used by CESM to create env_postprocessing.xml. Variables in this XML file are then used to generate the cupid config file for CESM cases
1. Bring in latest ADF commit as external
2. generate_cupid_config_for_cesm_case.py now has --adf-output-root argument
that lets users specify a place other than ${CASEROOT} for ADF_output/
Also cleaned up comments about start_years and end_years in timeseries.py
Keeping with CESM convention, using STARTDATE instead of START_YEAR. This propogates into generate_cupid_config_for_cesm_case.py, which wants to drop the MM-DD portion of the date in favor of saving just the year for timeseries generation (where we assume ENDDATE = YYYY-01-01, so END_YEAR = YYYY - 1)
When you create a new case, CESM will create case.cupid, which in turn will execute the new cesm_postprocessing.sh script -- this means we can update what CUPiD does in the CESM workflow without requiring changes to ccs_config_cesm
|
This is ready for a final review -- the final summary of changes:
Happy to address any concerns in the implementation, but I think this has all the features we need for |
There was a problem hiding this comment.
This approval seems awfully shady, since half the commits in this PR are from me... but @TeaganKing can't approve it because she opened the PR (and also made the other half of the commits) 😄
Hah! I am reviewing this right now, too. I will be commenting on it shortly... |
TeaganKing
left a comment
There was a problem hiding this comment.
I have just a few minor change requests.
| echo "CUPID_RUN_ALL is True, running diagnostics for all components" | ||
| fi | ||
|
|
||
| unset PYTHONPATH |
There was a problem hiding this comment.
It may make more sense to move this closer to the comment on line 21 (or move the comment) so long as that doesn't mess anything else up?
There was a problem hiding this comment.
Good catch! It seems safer to move the comment down than the unset command up :)
| cupid_startdate : str | ||
| The start date of the case being analyzed ("YYYY-MM-DD"). | ||
|
|
||
| cupid_enddate : int |
There was a problem hiding this comment.
I think this one is a str?
There was a problem hiding this comment.
Yup, another good catch :)
1. Move comment about unsetting python path to where we unset the path 2. correct comment about cupid_enddate datatype Note: I also removed the ./xmlquery MACH call because we weren't using that anywhere (I wonder if we used to only run some commands based on being on derecho, but now it's all controlled by env_postprocessing.xml)
|
@TeaganKing hopefully ed9c322 addresses both of your requests (the |
|
Great! Looks good to me! |
|
|
||
| <!-- Variables that define which components are analyzed --> | ||
|
|
||
| <entry id="CUPID_RUN_ALL"> |
There was a problem hiding this comment.
Not a high priority, but do all of these need to be CUPID_RUN_STUFF?
There was a problem hiding this comment.
I think the idea was that it would be clear that these are cupid-specific variables that turn on running a particular component. So, I wouldn't want to just leave the variable name as only ATM or ICE, for instance. Open to other suggestions, though!
There was a problem hiding this comment.
I think we should leave the variable names as-is for now, we can always change them later :)
| "base_case_output_dir = None # None => use CESM_output_dir\n", | ||
| "case_name = \"\" # \"b.e30_beta02.BLT1850.ne30_t232.104\"\n", | ||
| "base_case_name = \"\" # \"b.e23_alpha17f.BLT1850.ne30_t232.092\"\n", | ||
| "base_case_name = None # \"b.e23_alpha17f.BLT1850.ne30_t232.092\"\n", |
Description of changes: This PR will include additional parameters in the helper script which generates the cupid configuration file to account for a more flexible cupid workflow.
pre-commitchecks passed (#8 in Adding Notebooks Guide)?