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

Add NLDAS grid for CTSM and MOSART #3063

Merged
merged 7 commits into from
Apr 16, 2019
Merged

Conversation

billsacks
Copy link
Member

Adds a high-resolution regional grid over the U.S. for use in land-only
simulations.

(I haven't added the necessary files to the inputdata repository yet,
but will do so once this and the corresponding CTSM PRs are accepted.)

Test suite: SMS test using this new grid in the context of a CTSM branch
with the necessary changes
Test baseline: n/a
Test namelist changes: none
Test status: bit for bit

Fixes none

User interface changes?: N

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

Code review:

It would be hard for someone to remember the right number of grid points
to use; this new alias is easier to remember.
@billsacks billsacks added ty: enhancement Responsibility: CESM Responsibility to manage and accomplish this issue is through CESM tp: config labels Apr 4, 2019
@billsacks billsacks self-assigned this Apr 4, 2019
@billsacks billsacks requested a review from mvertens April 4, 2019 12:36
@@ -120,6 +120,15 @@
<grid name="rof">null</grid>
</model_grid>

<!-- Regional NLDAS grid over the U.S. (roughly 12km resolution), with the standard NLDAS mask -->
<model_grid alias="nldas_mnldas" compset="DATM.+CLM">
Copy link
Member Author

Choose a reason for hiding this comment

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

Does this seem like a reasonable name for the alias? I struggled a bit, because the mask has the same name as the grid: we're using the standard NLDAS mask with the NLDAS grid.

For many grid aliases, we also give the ocean resolution, but that seems irrelevant here, and would lead to even more redundancy in the name (nldas_nldas_mnldas).

But I'm very open to suggestions here to keep this consistent with any grid naming conventions.

Copy link
Contributor

Choose a reason for hiding this comment

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

why nldas instead of 12km or nl12?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, @jedwards4b . Though a couple of thoughts on this are:

  • I prefer spelling things out a bit more, so would probably go with nldas12_mnldas or nldas12km_mnldas
  • Do you think that including 12 or 12km in the name gives the impression that the grid is at exactly 12 km (which isn't true)? This grid is actually at 1/8 deg – so roughly, but not exactly, 12 km.

@barlage I'd also welcome any opinions you have on this.

Copy link

Choose a reason for hiding this comment

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

@billsacks My preference would be to keep this as nldas_mnldas and leave the details in the grid definition below; NLDAS is a well-defined grid but another consideration would be to add an NLDAS version here (e.g. nldas2) since I believe NLDAS v3 may have a different, higher resolution grid when released. Is there any need to explicitly declare this as regional in the model_grid?

<grid name="atm">224x464_nldas</grid>
<grid name="lnd">224x464_nldas</grid>
<grid name="ocnice">224x464_nldas</grid>
<grid name="rof">null</grid>
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm using the same mechanism we have used elsewhere to turn off ROF for this regional grid: setting the rof grid to null.

@@ -120,6 +120,15 @@
<grid name="rof">null</grid>
</model_grid>

<!-- Regional NLDAS grid over the U.S. (roughly 12km resolution), with the standard NLDAS mask -->
<model_grid alias="nldas_mnldas" compset="DATM.+CLM">
Copy link
Contributor

Choose a reason for hiding this comment

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

why nldas instead of 12km or nl12?

@@ -120,6 +120,15 @@
<grid name="rof">null</grid>
</model_grid>

<!-- Regional NLDAS grid over the U.S. (roughly 12km resolution), with the standard NLDAS mask -->
Copy link

@barlage barlage Apr 4, 2019

Choose a reason for hiding this comment

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

Suggestion:

Regional NLDAS-2 grid over the U.S. (0.125 degree resolution; 25-53N, 235-293E), with the standard NLDAS-2 mask

@@ -120,6 +120,15 @@
<grid name="rof">null</grid>
</model_grid>

<!-- Regional NLDAS grid over the U.S. (roughly 12km resolution), with the standard NLDAS mask -->
<model_grid alias="nldas_mnldas" compset="DATM.+CLM">
<grid name="atm">224x464_nldas</grid>
Copy link

Choose a reason for hiding this comment

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

is it too much to add the resolution here, e.g., 224x464_0.125_nldas2?

@billsacks billsacks changed the title Add NLDAS grid for CTSM DO NOT MERGE: Add NLDAS grid for CTSM Apr 4, 2019
@billsacks billsacks removed the request for review from mvertens April 4, 2019 21:16
@billsacks
Copy link
Member Author

Based on comments today, I need to rework this. I'll comment when it's ready for review.

Based on discussion in Thursday's CTSM software meeting
@billsacks
Copy link
Member Author

I have updated this PR with new grid and mask names based on discussions with @barlage and @mvertens . I plan to make files using this new naming convention later today, so please speak up soon if you have problems with these names.

After that, I'll still need to:

  • Make new domain files using the new mask definition that we discussed on Thursday (using the mask of the NLDAS atmospheric forcing data, so it will include lakes)
  • Add grid and mapping files for a corresponding 1/8 deg regional ROF grid

These:
- Use a land mask based on the nldas2 atmospheric forcing data, covering
  more points than we had been using before (this mask includes lakes
  and some additional coastal points)
- Use the new grid name
@billsacks billsacks mentioned this pull request Apr 14, 2019
Since the rof grid is identical to lnd, we should be able to use idmap
for the rof <-> lnd mappings.
@billsacks billsacks changed the title DO NOT MERGE: Add NLDAS grid for CTSM Add NLDAS grid for CTSM and MOSART Apr 16, 2019
@billsacks billsacks requested a review from mvertens April 16, 2019 15:21
@billsacks billsacks assigned jedwards4b and unassigned billsacks Apr 16, 2019
@billsacks
Copy link
Member Author

Okay, this is ready for final review. @mvertens @jedwards4b @barlage do you want to look at this one last time, with particular focus on the grid names (full grid name and alias)?

Note that the ROF grid is identical to the LND grid, so no mapping files are needed.

@billsacks billsacks removed the request for review from mvertens April 16, 2019 16:53
@billsacks
Copy link
Member Author

@mvertens gave her approval by email.

@jedwards4b jedwards4b merged commit a356310 into ESMCI:master Apr 16, 2019
@billsacks billsacks deleted the nldas_grid_v2 branch April 16, 2019 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Responsibility: CESM Responsibility to manage and accomplish this issue is through CESM tp: config ty: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants