Physgrid: Introduce new topography file format. #3267

ambrad · 2019-10-29T22:12:53Z

Improve treatment of topography for physgrid configurations. Better GLL/PG consistency and error checking when reading netcdf files.

[BFB]

I. Background. Topography data include phi_s, SGH, SGH30, and landm_coslat. SGH and SGH30 area(Omega)^-1 sqrt( integral_Omega (h1(x) - h2(x))^2 dx ), where h1 and h2 are two topography references. SGH captures the smoothing between an ~5-km length scale and the actual grid. SGH30 captures the smoothing between the ~5-km length scale and the true (30") topography. 1. We want phi_s on the dynamics grid and the FV grid to be consistent in the sense that an integral over an FV subcell of either representation gives the same value. 2. We also what topography to be as rough as the simulation can tolerate. 1 is currently true because we read FV PHIS, remap to GLL, then remap back to FV to make 1 true. But this procedure does not satisfy 2 because it introduces new smoothing. II. New topo file format, the mixed GLL-FV format. Thus, a new topography file format will have the fields PHIS_d, phi_s on the GLL grid PHIS, phis_s on the FV grid SGH, SGH30, LANDM_COSLAT, LANDFRAC on the FV grid The old file format, which does not contain PHIS_d, is still supported. III. Final tool chain. PHIS_d will have exactly the smoothness the dycore requires. Then PHIS_d is remapped to PHIS. The other fields can be computed from PHIS. This requires cube_to_target and other tools. IV. Converter. This PR provides a tool in standalone Homme to convert an old GLL file to a new mixed GLL-FV file. The remapped SGH is augmented with additional variance due to the remap. The results are good, but the final tool chain should be used for new grid development work.

ambrad · 2019-10-29T22:13:50Z

e3sm_developer testing in progress.

See Topography section on the physgrid results page for details.

whannah1 · 2019-10-30T14:03:36Z

components/cam/src/dynamics/se/inidat.F90

@@ -451,30 +452,45 @@ subroutine read_inidat( ncid_ini, ncid_topo, dyn_in)
          end do
    end do

+    read_pg_grid = .false.


There's very likely going to be a conflict between this and PR #3204. It might be a good idea to merge those commits in to this branch.

This PR will go in after that one, so I'll merge master at the end when this PR is ready.

ok, that works. The conflicts shouldn't be too hard to fix since the block isn't that big.

whannah1 · 2019-10-30T14:20:20Z

components/cam/src/dynamics/se/inidat.F90

-            call fv_phys_to_dyn_topo(elem,phys_tmp)
+         call fv_phys_to_dyn_topo(elem,phys_tmp)
+      else
+         ! Attempt to read a mixed GLL-FV topo file, which contains PHIS_d in


I think we need a better description of what happens when the topo is on the GLL grid. Maybe I'm behind on this discussion, but last time we talked about it I thought we were sure that the topo used by the dynamics needs to be defined on the FV grid for the purpose of the the parameterizations that handle the "sub-grid" topography that live in physics world.
Is this change just a convenience so we don't have to create the topo on the FV grid? In other words, it allows us to use the conventional smoothing of topo on the GLL grid rather than defining a new smoothing operator on the FV grid?

First, let me quote a commit message so we have this information available in the conversation:

I. Background. Topography data include phi_s, SGH, SGH30, and landm_coslat. SGH
and SGH30
area(Omega)^-1 sqrt( integral_Omega (h1(x) - h2(x))^2 dx ),
where h1 and h2 are two topography references.

SGH captures the smoothing between an ~5-km length scale and the actual
grid. SGH30 captures the smoothing between the ~5-km length scale and the true
(30") topography.

We want phi_s on the dynamics grid and the FV grid to be consistent in the
sense that an integral over an FV subcell of either representation gives the
same value.

We also what topography to be as rough as the simulation can tolerate.

1 is currently true because we read FV PHIS, remap to GLL, then remap back to FV
to make 1 true. But this procedure does not satisfy 2 because it introduces new
smoothing.

II. New topo file format, the mixed GLL-FV format. Thus, a new topography file
format will have the fields
PHIS_d, phi_s on the GLL grid
PHIS, phis_s on the FV grid
SGH, SGH30, LANDM_COSLAT, LANDFRAC on the FV grid
The old file format, which does not contain PHIS_d, is still supported.

III. Final tool chain. PHIS_d will have exactly the smoothness the dycore
requires. Then PHIS_d is remapped to PHIS. The other fields can be computed from
PHIS. This requires cube_to_target and other tools.

IV. Converter. This PR provides a tool in standalone Homme to convert an old GLL
file to a new mixed GLL-FV file. The remapped SGH is augmented with additional
variance due to the remap. The results are good, but the final tool chain should
be used for new grid development work.

the topo used by the dynamics needs to be defined on the FV grid

Yes.

Is this change just a convenience so we don't have to create the topo on the FV grid?

No; see below.

In other words, it allows us to use the conventional smoothing of topo on the GLL grid rather than defining a new smoothing operator on the FV grid?

Yes, but that is not a matter of convenience, IMO. Rather, the dycore needs a specific type of smoothing, smoothing specific to the GLL grid. So this new file format provides exactly these GLL PHIS_d data. PHIS is then PHIS_d mapped to the FV grid, which gives the least amount of smoothing possible on both grids while keeping GLL and FV phis_s consistent. Then cube_to_target can run using PHIS (not PHIS_d), giving the FV SGH, SGH30, and landm_coslat.

This PR does have a convenience feature, however. The converter produces a file in this new format from an old pure-GLL topo file. A future PR will fill in the tool chain for doing each calculation on the right grid.

ok, this is fantastic. I like the new approach.

Does this mean the dual grid is no longer needed?

I believe adoption of (1) physgrid and (2) tri-grid will make the GLL dual grid unnecessary in v2. This PR does not change the status of the dual grid; it was already unnecessary for physgrid topo. This PR focuses on maximizing the quality of topo on each grid while keeping the two topo representations consistent, which does not require the GLL dual grid.

SCREAM may not use tri-grid, in which case the GLL dual grid will still be needed for that project. But I believe the status of that point is uncertain.

components/homme/src/tool/tool_main.F90

mt5555 · 2019-10-31T00:27:31Z

components/homme/CMakeLists.txt

@@ -101,6 +101,7 @@ OPTION(BUILD_HOMME_PREQX_ACC "Primitive equations FEM with OpenACC" ON)
 OPTION(BUILD_HOMME_PREQX_KOKKOS "Primitive equations FEM with Kokkos" OFF)
 OPTION(BUILD_HOMME_SWIM  "Shallow water equations implicit" OFF)
 OPTION(BUILD_HOMME_PRIM  "Primitive equations implicit" OFF)
+OPTION(BUILD_HOMME_TOOL  "Offline tool" OFF)


any reason not to make this default ON?

No reason. Will do.

mt5555 · 2019-10-31T00:31:06Z

nice PR. I'm looking forward to moving the interpolate, template and toposmoothing tools into the new tool framework.

ambrad · 2019-10-31T01:13:25Z

This PR should go in only after #3204 and #3251. Once those go in, I'll merge master and resolve the expected conflicts in inidat.F90.

mt5555 · 2019-10-31T01:25:40Z

@whannah1 : what's the status of #3204 - i seem to remember it was delayed because of some CIME issue. should we really delay this PR until afer #3204?

whannah1 · 2019-10-31T14:22:39Z

@mt5555 I was just waiting for that extra review from @AaronDonahue for #3204.
For #3251, we should talk about whether to also include removing the low-order mapping, since it would make the original intention of #3251 moot.

oksanaguba · 2019-10-31T16:40:13Z

components/homme/src/tool/CMakeLists.txt

+SET(UTILS_SHARE_DIR   ${HOMME_SOURCE_DIR}/utils/csm_share)
+SET(SRC_DIR           ${HOMME_SOURCE_DIR}/src)
+SET(SRC_SHARE_DIR     ${HOMME_SOURCE_DIR}/src/share)
+SET(TEST_SRC_DIR      ${HOMME_SOURCE_DIR}/src/test_src)


Probably tools won't need test_src?

Test modules get used in various spots in the code, so it's needed to build.

this is very minor: I am only asking because there is a code in tests_src that needs commenting if one wants to run intel with checks, for example. I thought tools won't need tests, so, it would make debugging easier. or maybe build tools with CAM=true directive.

You might open an issue about "code in tests_src that needs commenting if one wants to run intel with checks," as that implies something is broken.

What I meant above is that there is core code that uses test_mod, which in turn uses modules in test_src. So new cpp code would be needed to isolate test_src sufficiently. I consider that orthogonal to this PR. I believe setting CAM to true would create unresolved uses of modules from CAM.

oksanaguba · 2019-10-31T16:42:08Z

components/homme/src/tool/CMakeLists.txt

+IF (NOT PREQX_PLEV)
+  SET (PREQX_PLEV 20) 
+ENDIF ()
+IF (NOT PREQX_USE_PIO)


I would assume most of tools would need PIO=TRUE here (native input/output)...

Good point.

ambrad · 2019-10-31T19:31:36Z

I'm running e3sm_developer again b/c I had not sync'ed my repo's master and my branch after the recent IMEX PR. In my e3sm_dev run that just finished, all tests except HOMME's standalone theta-l tests pass.

Edit: Now all tests pass.

ambrad · 2019-11-11T17:56:41Z

Now that PR #3204 in in master, I'll merge master into this branch, resolve the inidat conflict, and rerun tests.

…id-new-topo-file-format Conflicts: components/cam/src/dynamics/se/inidat.F90 Merge master to get PR E3SM-Project#3204; resolve the conflict in inidat concerning reading PHIS(_d) from the topo file.

ambrad · 2019-11-11T23:27:20Z

All e3sm_developer tests pass.

Improve treatment of topography for physgrid configurations. Better GLL/PG consistency and error checking when reading netcdf files. [BFB]

Improve treatment of topography for physgrid configurations. Better GLL/PG consistency and error checking when reading netcdf files. [BFB

ambrad and others added 3 commits October 29, 2019 15:59

Physgrid: Modify inidat to handle either topo file format.

c6d7a83

better nlev checking when reading netcdf files

485dfd3

ambrad added enhancement Atmosphere BFB PR leaves answers BFB labels Oct 29, 2019

ambrad requested a review from whannah1 October 29, 2019 22:12

ambrad assigned mt5555 Oct 29, 2019

ambrad requested a review from oksanaguba October 29, 2019 22:20

whannah1 reviewed Oct 30, 2019

View reviewed changes

components/homme/src/tool/tool_main.F90 Show resolved Hide resolved

mt5555 reviewed Oct 31, 2019

View reviewed changes

mt5555 approved these changes Oct 31, 2019

View reviewed changes

oksanaguba reviewed Oct 31, 2019

View reviewed changes

Physgrid: Respond to reviews; fix inidat _d suffix bug.

1eb3060

whannah1 mentioned this pull request Nov 1, 2019

Fix physgrid issues with PE layout and aqua restarts #3204

Merged

ambrad added 2 commits November 11, 2019 12:08

Merge remote-tracking branch 'upstream/master' into ambrad/eam/physgr…

d18cd46

…id-new-topo-file-format Conflicts: components/cam/src/dynamics/se/inidat.F90 Merge master to get PR E3SM-Project#3204; resolve the conflict in inidat concerning reading PHIS(_d) from the topo file.

EAM: Fix a variable name associated with the merge conflict.

691921a

HOMME: Fix shape of LHS of assignment.

deeb24c

mt5555 added a commit that referenced this pull request Dec 4, 2019

Merge ambrad/eam/physgrid-new-topo-file-format into next (PR #3267)

d7cef07

Improve treatment of topography for physgrid configurations. Better GLL/PG consistency and error checking when reading netcdf files. [BFB]

mt5555 added a commit that referenced this pull request Dec 5, 2019

Merge ambrad/eam/physgrid-new-topo-file-format (PR #3267)

f3f1ace

Improve treatment of topography for physgrid configurations. Better GLL/PG consistency and error checking when reading netcdf files. [BFB

mt5555 merged commit deeb24c into E3SM-Project:master Dec 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Physgrid: Introduce new topography file format. #3267

Physgrid: Introduce new topography file format. #3267

ambrad commented Oct 29, 2019 •

edited by mt5555

ambrad commented Oct 29, 2019

whannah1 Oct 30, 2019

ambrad Oct 30, 2019

whannah1 Oct 30, 2019

whannah1 Oct 30, 2019

ambrad Oct 30, 2019

ambrad Oct 30, 2019

whannah1 Oct 30, 2019

rljacob Oct 31, 2019

ambrad Oct 31, 2019

mt5555 Oct 31, 2019

ambrad Oct 31, 2019

mt5555 commented Oct 31, 2019

ambrad commented Oct 31, 2019

mt5555 commented Oct 31, 2019

whannah1 commented Oct 31, 2019

oksanaguba Oct 31, 2019

ambrad Oct 31, 2019

oksanaguba Oct 31, 2019

ambrad Oct 31, 2019

oksanaguba Oct 31, 2019

ambrad Oct 31, 2019

ambrad commented Oct 31, 2019 •

edited

ambrad commented Nov 11, 2019

ambrad commented Nov 11, 2019

Physgrid: Introduce new topography file format. #3267

Physgrid: Introduce new topography file format. #3267

Conversation

ambrad commented Oct 29, 2019 • edited by mt5555

ambrad commented Oct 29, 2019

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

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

mt5555 commented Oct 31, 2019

ambrad commented Oct 31, 2019

mt5555 commented Oct 31, 2019

whannah1 commented Oct 31, 2019

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

ambrad commented Oct 31, 2019 • edited

ambrad commented Nov 11, 2019

ambrad commented Nov 11, 2019

ambrad commented Oct 29, 2019 •

edited by mt5555

ambrad commented Oct 31, 2019 •

edited