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.

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

Neon compsets #1278

Merged
merged 87 commits into from
May 18, 2021
Merged

Neon compsets #1278

merged 87 commits into from
May 18, 2021

Conversation

jedwards4b
Copy link
Contributor

@jedwards4b jedwards4b commented Feb 9, 2021

Description of changes

Adds support for NEON tower data sites.

Also bring in initial version of subset_data.py to create surface datasets from the global datasets for a particular site.

Update externals to close to the version in cesm2_3_alpha03a

Specific notes

Contributors other than yourself, if any: @ekluzek @negin513

CTSM Issues Fixed (include github issue #):
Fixes #1368
Fixes #1353

Are answers expected to change (and if so in what way)? Only for cases running with CISM (compset aliases ending in G)

Any User Interface Changes (namelist or namelist defaults changes)? Add NEONSITE to env_run.xml
surface datasets for NEON sites in the NEON user-mod-directories

Testing performed, if any: regular
cheyenne
izumi (note that NUOPC tests are now failing to build on izumi)

@jedwards4b jedwards4b self-assigned this Feb 9, 2021
@jedwards4b jedwards4b marked this pull request as draft February 9, 2021 15:40
@billsacks
Copy link
Member

@jedwards4b I'm not sure if this was intentional, but it looks like you based this branch off of Mariana's nuopc development branch, which contains a lot of features that haven't yet come to CTSM master (and which Mariana has asked me to wait to review & integrate until she has a chance to do some more work & validation on it). If there's a reason for your changes to be built on top of those, then we can wait to review this branch until that one comes in (at which point this one will show many fewer differences), but if your work is unrelated to that, and if you want us to review this sooner, then it could be best if you rebase your commits onto a branch off of master.

@jedwards4b
Copy link
Contributor Author

My work is related in as much as it uses cmeps and cdeps. I don't think this is urgent.

@ekluzek ekluzek added tag: enh - new science enhancement new capability or improved behavior of existing capability labels Feb 10, 2021
@billsacks
Copy link
Member

billsacks commented Feb 10, 2021

Sounds good, @jedwards4b - thanks. And thank you for your work on this!

In looking through the latest commits, I see a lot of whitespace changes in namelist_defaults_ctsm.xml. @ekluzek and my general feeling is that whitespace fixes in the vicinity of changes are fine, but we try to avoid combining big whitespace cleanups like this with actual changes, because it can create merge conflicts that can be time consuming and error-prone for others to resolve.

  • Can you please either remove the whitespace changes in namelist_defaults_ctsm.xml or give me the go-ahead to remove these whitespace changes myself?

Let me know if you'd like to talk further about this. (For example, I can share my emacs setup for cleaning whitespace on my own changes without affecting other parts of the file, if you're interested.)

@jedwards4b
Copy link
Contributor Author

Please feel free to replace any unnecessary white space that I removed.

@ekluzek
Copy link
Collaborator

ekluzek commented Feb 10, 2021

Glad to see this work is started and moving. NEON is a cool project.

I have comments that are similar to what I put in the similar PR on cime...

ESMCI/cime#3850

The main complaint is that I think it's problematic to have surface datasets with generic names, and hence whose content can change underneath others. So I became convinced that having user-mod directories for this type of thing is a better way to handle it.

I'm also not convinced you need a finidat file for NEON sites? It should pull you a global version that can be used. But, perhaps this is being done to pull the nearest gridpoint? @jedwards4b clarified that this is needed so that you have a spunup IC file for each site once those are developed. Since, getting a site spunup is a big deal we'll want to save the spunup states for each site.

As above I'm not completely sure if you need NEON_SITE separate from CLM_USRDAT_NAME. But, I also see that you have an explicit list of NEON sites and that is good to have so the names are correct. If you can give a list of names it's the best way to do error checking, so I do really like that. So it might be the best approach would be to have some mix of the two. I think we might want to have some discussions over the best way to get this all setup.

@ekluzek
Copy link
Collaborator

ekluzek commented Feb 18, 2021

Making sure @danicalombardozzi and @wwieder as well as @negin513 know about @jedwards4b pull request. Also see the cime issue that this points to.

@ekluzek
Copy link
Collaborator

ekluzek commented Feb 19, 2021

Another CTSM issue that relates to this is #1198

@danicalombardozzi
Copy link
Contributor

danicalombardozzi commented May 11, 2021

Documenting the required steps for testing and running a NEON tower simulation so that they are available to others (I don't think they're documented anywhere):

  1. Clone relevant code from Git (this will be on master soon, currently requires checking out Jim's branch)

  2. In the clone, check out the NEON compsets: git checkout neon_compsets (this must happen before checking out externals)

  3. After checking out externals, set up a simulation:
    ./create_newcase --case CASENAME --compset I1PtClm51Bgc --res CLM_USRDAT --driver nuopc --user-mods NEON/select_site

  4. Right now, getting input data requires Jim's fix from above:
    cd /glade/scratch/$USER
    mkdir inputdata
    cd inputdata
    ln -fs /glade/p/cesmdata/cseg/inputdata/* .
    rm atm
    mkdir -p atm/cdeps

cd $CASEROOT
./xmlchange DIN_LOC_ROOT=/glade/scratch/$USER/inputdata

Other notes:
This is really great! It's so easy to set up a tower site simulation with the changes that have been made!

I know we are planning to update the surface dataset, and I wanted to confirm that the initial conditions are (or will be) spun up for each site.

If I understand correctly, the steps required in 4 above will not be required by the user (code will be updated).

One additional thing that stands out: The simulation is set to run for 5 days by default (in env_run). Can (and should) we update this to run over the length of time that the atmospheric forcing is available for? I'm not sure how people will know how long they can run the simulation otherwise, unless they know to look at the datm.streams.xml file.

@jedwards4b
Copy link
Contributor Author

jedwards4b commented May 11, 2021 via email

@danicalombardozzi
Copy link
Contributor

step 4 is only required if the user does not have write access to DIN_LOC_ROOT on their system. I am not planning any further changes to address this.

Ok, I think I understand this. It should work for someone running the simulation on their laptop, but it doesn't work for people without cseg write access on Cheyenne.

If this is indeed the case, we'll need to keep these instructions around for people who are running the simulations on Cheyenne (likely just folks in TSS, but maybe other collaborators who have access).

@ekluzek ekluzek added the PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete label May 14, 2021
@wwieder wwieder mentioned this pull request May 14, 2021
7 tasks
@ekluzek ekluzek merged commit c977520 into ESCOMP:master May 18, 2021
@samsrabin samsrabin added the science Enhancement to or bug impacting science label Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new capability or improved behavior of existing capability PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete science Enhancement to or bug impacting science
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Additional modifications to subset_data.py Modify NEON surface data
7 participants