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 datm mode #3850

Closed
wants to merge 3 commits into from
Closed

Neon datm mode #3850

wants to merge 3 commits into from

Conversation

jedwards4b
Copy link
Contributor

Adds a 1x1 grid for NEON data sites. This is used with ctsm and cdeps to generate single column clm cases with this data.

Test suite: hand testing, scripts_regression_tests.py
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes

User interface changes?:

Update gh-pages html (Y/N)?:

Code review:

@jedwards4b jedwards4b requested a review from ekluzek February 9, 2021 14:49
@jedwards4b jedwards4b self-assigned this Feb 9, 2021
@jedwards4b jedwards4b marked this pull request as draft February 9, 2021 14:50
@jedwards4b jedwards4b mentioned this pull request Feb 9, 2021
12 tasks
Copy link
Contributor

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

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

@jedwards4b glad to see this work is getting started. I request some changes that come about because of previous work I've done where I found problems with a similar approach that I took as well. It likely might be good to chat about this and maybe include Will on the discussion. NEON is a cool project and it will be really awesome to get their data integrated into the model.

@@ -110,6 +110,12 @@
<grid name="rof">null</grid>
</model_grid>

<model_grid alias="1x1_NEON" compset="DATM.+CLM|DATM.+SLND">
Copy link
Contributor

Choose a reason for hiding this comment

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

It appears that 1x1_NEON is doing the same thing as CLM_USRDAT_NAME, I'm not sure we need an additional name here that's specific for NEON.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also suggest when using 1x1_ to use 1x1pt_ as used in other places since the default units for grids are in degrees, and the "pt" says that in this case it's "points". So 1x1pt_ is a single point regional grid, while something like 5x5pt_ is a grid that's has 5 points in latitude and also 5 in longitude.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you look at the line below you will see the reason for this name, it supports all of the NEON tower sites:
$DIN_LOC_ROOT/share/domains/single_point/datmdata_NEON/$NEONSITE/domain.lnd.fv
0.9x1.25_gx1v7_$NEONSITE.nc

How about we change the file name to: domain.lnd.1x1pt_$NEONSITE_20210210.nc

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, since these files shouldn't change (except maybe adding new sites?) I think that solution makes perfect sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

The one other question I had about this though is I though @mvertens was doing something where single point sites didn't require domain files anymore for NUOPC? I'd have to look at it again to see how that worked though...

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I looked at that again, and what she was doing was to eliminate the need for both mesh files AND domain files. But, it's still configured to need domain files.

@@ -1306,6 +1312,13 @@
<desc>user specified domain - only valid for DATM/CLM compset</desc>
</domain>

<domain name="1x1_NEON">
<nx>1</nx> <ny>1</ny>
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem here that I ran into with CLM_USRDAT_NAME is that this domain file when updated will change ALL existing simulations being run with it. This is problematic. As such I came to the conclusion a better way to handle it is to have scripts that setup single-point create user-mod directories where the DOMAIN file is set to a particular path that includes a time-stamp in the filename. This way changes to files don't change the results of someone else's existing simulations.

@ekluzek
Copy link
Contributor

ekluzek commented Feb 10, 2021

A relevant issue in CTSM is this one ESCOMP/CTSM#1041 where I decided that having specific filenames hardcoded into CTSM wasn't useful (having CLM_USRDAT_NAME still is useful), and using user-mod directories was a better approach.

An example user-mod directory put together for a single-point case is here:

https://svn-ccsm-inputdata.cgd.ucar.edu/trunk/inputdata//lnd/clm2/PTCLMmydatafiles.c171024/1x1pt_US-UMB

PTCLM is being deprecated, but I think the above kind of user-interface for what it does still makes sense. With the creation of these types of directories you can create a case with a single added option added to create_newcase. And that case won't suddenly have answer changes because someone else updated files that have generic names without time-stamps.

@jedwards4b
Copy link
Contributor Author

superceeded

@jedwards4b jedwards4b closed this Apr 16, 2021
jedwards4b added a commit to ESCOMP/CDEPS that referenced this pull request Apr 23, 2021
### Description of changes
Adds support for NEON tower data in datm.

### Specific notes

Contributors other than yourself, if any:

CMEPS Issues Fixed (include github issue #):

Are there dependencies on other component PRs
 - [X] CIME (list)   Requires branch neon_data_mode (PR ESMCI/cime#3850)
 - [ ] CMEPS (list) 

Are changes expected to change answers?
 - [X] bit for bit
 - [ ] different at roundoff level
 - [ ] more substantial 

Any User Interface Changes (namelist or namelist defaults changes)?
 - [ ] Yes
 - [X] No

Testing performed:
- [X] (required) aux_cdeps
   - machines and compilers: cheyenne intel 
   - details (e.g. failed tests): SMS_Vnuopc_Ld5.TL319_t061.2000_DATM%JRA-1p4-2018_SLND_SICE_SOCN_SROF_SGLC_SWAV_SESP.cheyenne_intel   fails in build with
       Invalid values ['CORE_IAF_JRA_1p4_2018']
- [ ] (optional) CESM prealpha test
   - machines and compilers
   - details (e.g. failed tests):

Hashes used for testing:
- [ ] CIME
  - repository to check out: https://github.com/ESCOMP/CESM.git
  - branch:  master
  - hash:8b48a2e
- [ ] CMEPS
  - repository to check out: https://github.com/ESCOMP/CESM.git
  - branch: master
  - hash: v0.9.0
- [ ] CESM
  - repository to check out: https://github.com/ESCOMP/CESM.git
  - branch: master
  - hash:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants