Support cesm workflow #131

mnlevy1981 · 2024-09-07T15:36:09Z

Code changes that will be useful when running CUPiD via the CESM workflow.

All Submissions:

Have you followed the guidelines in our Contributor's Guide (including the pre-commit check)?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

Does your submission pass tests?
Have you lint your code locally prior to submission?

Changes to Core Features:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you successfully tested your changes locally?

This is the script that generates a CUPiD config file based on an existing yaml file. I'm committing it as it was in Ingrid's cesm2_3_beta17 sandbox, for use with CUPiD at hash b402eba; I think it will need to be updated for more recent versions.

generate_cupid_config_file.py now relies on argparse to get a handful of commandline arguments instead of importing some CIME functions and determining these values on the fly (it'll be easy to set all these arguments when creating the template)

mnlevy1981 · 2024-09-08T18:52:40Z

I added the generate_cupid_config_file.py that @ingridc2051 wrote to CUPiD, so that template.cupid in ESMCI/ccs_config_cesm#176 can find it. I made a couple small changes:

instead of importing some CIME functions, it expects CESM to pass command line arguments ($CASE, $DOUT_S_ROOT, and the location of CUPiD based off of $SRCROOT).
I added the sort_keys=False argument to yaml.dump so the resulting yaml file looks similar to the template (although missing the comments)

I'm not sure I like the location of the script, which is currently a new cesm_scripts/ subdirectory. I also think we need something more information than # sample comment in the file header... I can update that to something like # This file has been auto-generated for use with {{case}} based off of {{template file}} but I'm open to suggestions

TeaganKing · 2024-09-09T17:52:57Z

Can you clarify your reasoning for using command line arguments instead of importing CIME functions? I had originally thought the CIME functions were a bit more streamlined.
If we aren't going to be adding additional cesm scripts, I wonder if this could just go in the cupid directory?
I think that suggestion to update the comment is great.

mnlevy1981 · 2024-09-09T18:17:33Z

Can you clarify your reasoning for using command line arguments instead of importing CIME functions? I had originally thought the CIME functions were a bit more streamlined.

The original version of this code was placed in the CIME source tree, so it could easily import some CIME functions:

from standard_script_setup import *
from CIME.case import Case

Now that this script is owned by CUPiD, it would need a sys.path.append() call in front of those import statements, which would require a command line argument to pass in the path to these packages (or at least ${SRCROOT}). Also, we eventually want the user to be able to specify which CUPiD example should provide the base config.yml file.

I do think we could replace --cupid-root with --cesm-root (just adding an additional tools/CUPiD to the path), and then we could also import the CIME libraries and get both --case and --cesm-output-dir from the Case object. I'll work on that later this week (or maybe next week, a lot of this week is taken up by a workshop).

If we aren't going to be adding additional cesm scripts, I wonder if this could just go in the cupid directory?

I think we want to keep it separate, especially if we are going to pass --cesm-root as an argument. Also, the cupid/ directory should be reserved for key parts of the (cupid-dev) environment, while this script doesn't really require cupid-dev (it just needs to be run in an environment with pyyaml). Maybe the directory should be helper_scripts/ and this could be renamed generate_cupid_config_for_cesm_case.py?

I think that suggestion to update the comment is great.

👍

1. Only required command line argument is cesm root directory; also allows optional arguments for case directory (default is current dir) and cupid example (default is key_metrics) 2. Now gets case name and short-term archive directory from CESM Case class 3. Cleaned up comment inserted at top of config.yml

TeaganKing · 2024-09-09T23:23:38Z

Got it, that makes sense. The renaming looks good, too. I'll let you work on that replacement, and then can you please ping me again for review when it's ready?

mnlevy1981 · 2024-09-19T15:26:52Z

Got it, that makes sense. The renaming looks good, too. I'll let you work on that replacement, and then can you please ping me again for review when it's ready?

This one is ready for review (note the default directory for --case-root is whatever directory you run the script from, which I denote as {$PWD} but will actually appear in the help message as the full path):

$ ./generate_cupid_config_for_cesm_case.py -h
usage: generate_cupid_config_for_cesm_case.py [-h] [--cesm-root CESM_ROOT] [--cupid-example CUPID_EXAMPLE]
                                              [--case-root CASE_ROOT]

Generate config.yml based on an existing CUPID example

options:
  -h, --help            show this help message and exit
  --cesm-root CESM_ROOT
                        Location of CESM source code (default: None)
  --cupid-example CUPID_EXAMPLE
                        CUPiD example to use as template for config.yml (default: key_metrics)
  --case-root CASE_ROOT
                        CESM case directory (default: {$PWD})

helper_scripts/generate_cupid_config_for_cesm_case.py

TeaganKing · 2024-09-19T16:00:40Z

helper_scripts/generate_cupid_config_for_cesm_case.py

+        action="store",
+        dest="cupid_example",
+        default="key_metrics",
+        help="CUPiD example to use as template for config.yml",


The default as key_metrics is great. Is it worthwhile to mention that the example choices are coupled_model and key_metrics? I'm not sure how many different examples we'll end up having?

This is a tough one -- I'd love to use the choices argument of add_argument() along with the os.walk() function to list the subdirectories of CUPiD/examples/, but we don't know where the examples/ directory is until we resolve --cesm-root. The best option is probably to enforce this immediately after entering generate_cupid_config() by ensuring that os.path.join(cupid_root, "examples", cupid_example) is a directory and printing out a useful error message if it isn't?

That makes sense that adding the choices would entail using information we don't have yet. The suggestion you shared sounds great!

The code in bb3f37d has the check and gives the following:

$ ./generate_cupid_config_for_cesm_case.py --cesm-root ${CESM_ROOT} --case-root ${CESM_ROOT}/cases/test_cupid_workflow.007 --cupid-example foo Traceback (most recent call last): File "${CESM_ROOT}/tools/CUPiD/helper_scripts/./generate_cupid_config_for_cesm_case.py", line 107, in <module> generate_cupid_config(**args) File "${CESM_ROOT}/tools/CUPiD/helper_scripts/./generate_cupid_config_for_cesm_case.py", line 62, in generate_cupid_config raise KeyError( KeyError: "--cupid-example must be a subdirectory of ${CESM_ROOT}/tools/CUPiD/examples (['coupled_model', 'key_metrics'])"

Note that both ilamb and nblibrary are left off the list of valid_examples; eventually we'll probably reinstate ilamb, but for now it doesn't have a config.yml file. I'd really like to move nblibrary/ out of examples/ but that seems like a big thing to tackle in this PR :)

I should probably add text about 'foo' is not a valid option for --cupid-examples; I'll get to that in the morning

Sounds good!

Yes, let's move nblibrary out of examples after this PR is merged in.

0888657 has the improved check:

$ ./generate_cupid_config_for_cesm_case.py --cesm-root ${CESM_ROOT} --cupid-example foo Traceback (most recent call last): File "${CESM_ROOT}/tools/CUPiD/helper_scripts/./generate_cupid_config_for_cesm_case.py", line 108, in <module> generate_cupid_config(**args) File "${CESM_ROOT}/tools/CUPiD/helper_scripts/./generate_cupid_config_for_cesm_case.py", line 63, in generate_cupid_config raise KeyError( KeyError: "argument --cupid-example: invalid choice 'foo' (choose from subdirectories of ${CESM_ROOT}/tools/CUPiD/examples: ['coupled_model', 'key_metrics'])"

TeaganKing · 2024-09-19T16:03:42Z

helper_scripts/generate_cupid_config_for_cesm_case.py

+        "--case-root",
+        action="store",
+        dest="case_root",
+        default=os.getcwd(),


It may also be worth logging information on what arguments were used?

dba6bea adds information about the arguments to the comment section at the head of the new config.yml; I think that's the best place to put it but I'm open to other suggestions. (If we use a print() statement, then I think it will show up in the PBS log file generated by the case.cupid job, but other queue managers might treat that output differently?)

This seems like a good place-- especially since the config.yml file also says it's generated by the script, so all that information is in one area.

TeaganKing · 2024-09-19T16:36:35Z

Just a few minor comments, and otherwise this looks good to me!

generate_cupid_config_for_cesm_case.py now updates more global parameters (start_date, end_date, climo_nyears, as well as the base_ copies) and the time-series end_years. For now these values are hard-coded in with placeholders until we can get them from env_cupid.xml

I thought that not specifying a default value made it required, but really it just made the default None (which had all sorts of fun implications in the code)

We don't know the full list of valid values when parsing command line arguments, so this check needs to be done in the body of the python script

The header of the YAML file is a little bit bigger; in addition to mentioning the file was auto-generated, it includes each of the three arguments (cesm_root, case_root, and cupid_example)

TeaganKing

This looks great! Thanks for implementing this feature and making all those changes :)

mnlevy1981 added 3 commits September 7, 2024 09:22

Include script from I. Carlson

ac076aa

This is the script that generates a CUPiD config file based on an existing yaml file. I'm committing it as it was in Ingrid's cesm2_3_beta17 sandbox, for use with CUPiD at hash b402eba; I think it will need to be updated for more recent versions.

Clean-up to pass pre-commit checks

2b36874

mnlevy1981 marked this pull request as ready for review September 8, 2024 18:45

mnlevy1981 requested a review from TeaganKing September 8, 2024 18:46

mnlevy1981 added 2 commits September 9, 2024 16:49

Rename script and directory

a40ef5d

Merge branch 'main' into support_cesm_workflow

910e4c9

TeaganKing linked an issue Sep 18, 2024 that may be closed by this pull request

Move generate_cupid_config.py to CUPiD #119

Closed

TeaganKing reviewed Sep 19, 2024

View reviewed changes

mnlevy1981 added 5 commits September 19, 2024 15:35

generate_cupid_config... requires --cesm-root

a52a042

I thought that not specifying a default value made it required, but really it just made the default None (which had all sorts of fun implications in the code)

Add valid values for --cupid-example

bb3f37d

We don't know the full list of valid values when parsing command line arguments, so this check needs to be done in the body of the python script

Better error message for bad --cupid-example

0888657

Note script arguments in generated yaml file

dba6bea

The header of the YAML file is a little bit bigger; in addition to mentioning the file was auto-generated, it includes each of the three arguments (cesm_root, case_root, and cupid_example)

TeaganKing approved these changes Sep 20, 2024

View reviewed changes

mnlevy1981 merged commit c747ca4 into NCAR:main Sep 20, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support cesm workflow #131

Support cesm workflow #131

mnlevy1981 commented Sep 7, 2024 •

edited

Loading

mnlevy1981 commented Sep 8, 2024

TeaganKing commented Sep 9, 2024

mnlevy1981 commented Sep 9, 2024

TeaganKing commented Sep 9, 2024

mnlevy1981 commented Sep 19, 2024

TeaganKing Sep 19, 2024

mnlevy1981 Sep 19, 2024

TeaganKing Sep 19, 2024

mnlevy1981 Sep 19, 2024 •

edited

Loading

TeaganKing Sep 20, 2024

mnlevy1981 Sep 20, 2024

TeaganKing Sep 20, 2024

TeaganKing Sep 19, 2024

mnlevy1981 Sep 20, 2024

TeaganKing Sep 20, 2024

TeaganKing commented Sep 19, 2024

TeaganKing left a comment

Support cesm workflow #131

Support cesm workflow #131

Conversation

mnlevy1981 commented Sep 7, 2024 • edited Loading

All Submissions:

New Feature Submissions:

Changes to Core Features:

mnlevy1981 commented Sep 8, 2024

TeaganKing commented Sep 9, 2024

mnlevy1981 commented Sep 9, 2024

TeaganKing commented Sep 9, 2024

mnlevy1981 commented Sep 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnlevy1981 Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TeaganKing commented Sep 19, 2024

TeaganKing left a comment

Choose a reason for hiding this comment

mnlevy1981 commented Sep 7, 2024 •

edited

Loading

mnlevy1981 Sep 19, 2024 •

edited

Loading