Hommexx: Add doubly-periodic capability. #5481

ambrad · 2023-02-28T02:34:22Z

Implement doubly-periodic mode in Hommexx.
Add C++-vs-F90 tests for the F90 nhgw and bubble tests.

Fixes #4212 .

[BFB] except for standalone Homme F90 bubble tests; adds new standalone Homme tests

This also solves the multi-defs problem in the case of QSIZE_D == NUM_TIME_LEVELS.

* Implement doubly-periodic mode in Hommexx. * Add C++-vs-F90 tests for the F90 nhgw and bubble tests.

Previously, it was computing it according to the analytical formula. But that is not BFB in finite precision with the pressure_thickness call, which is the normal way to init dp3d in standalone tests. This was causing non-BFBness in the C++-vs-F90 test for the bubble tests. Differences in dp are at machine precision.

bartgol

Looks good. At some point it would be nice to add some more "intrinsic" tests, like basic grad/curl/div identities or lapl==div(grad(u)). I forgot if they (all) hold in curved manifolds, but they could at least be tested in planar geometries.

Obv, not for this pr.

oksanaguba · 2023-02-28T17:36:56Z

components/homme/src/test_src/dry_planar.F90

@@ -283,6 +282,10 @@ subroutine bubble_init(elem,hybrid,hvcoord,nets,nete,f)

  hvcoord%hyai = ai; hvcoord%hybi = bi

+  do k = 1,nlev


why this change?

Previously, it was computing it according to the analytical formula. But that is not BFB in finite precision with the pressure_thickness call, which is the normal way to init dp3d in standalone tests. This was causing non-BFBness in the C++-vs-F90 test for the bubble tests, since Hommexx's initialization assumes that dp3d is computed w.r.t. ps by the usual formula. Differences in dpm in bubble_init are at machine precision.

Ok, my first thought was to revert it. I do not believe we should stick to dcmip wrappers code. However if that it too much to change xx init, then it is ok to keep it. i think xx init should have mimic-ed F init though (i assume it is about routine set_elem_state() ).

The error, in my view, is in bubble_init. It sets up ps and dp3d in such a way that even if nothing happened in the first time step, dp3d would change (at machine precision) just due to using the standard formula in, e.g., vertical remap rather than bubble_init's computation. In contrast, all the other standalone tests use pressure_thickness for finite-precision consistency.

There is no reason to use A, B in dp3d away from init, and in init it is only a convenience. I do not see it as an error, however, it is a bigger change to fix this in xx, so let's keep this change.

Just for clarity, by consistency I mean e.g. with:

E3SM/components/homme/src/theta-l/share/vertremap_mod.F90

Lines 81 to 82 in c7b8bd8

dp(:,:,k) = ( hvcoord%hyai(k+1) - hvcoord%hyai(k) )*hvcoord%ps0 + &

( hvcoord%hybi(k+1) - hvcoord%hybi(k) )*elem(ie)%state%ps_v(:,:,np1)

If we want to remove a and b, it's not just an init-time issue or an Hxx issue.

I understand. At this point it is just a philosophical conversation about hybrid coefs, which we can postpone.

To ddcument this further, in Chrysalis, homme_integration shows diffs for the F90 bubble tests at machine precision level, consistent with the machine-precision change to dpm in bubble_init:

############################################################################ CPRNC returned the following RMS differences RMS T 7.0545E-13 NORMALIZED 2.4100E-15 RMS Th 4.4738E-14 NORMALIZED 1.4912E-16 RMS ps 9.2247E-11 NORMALIZED 9.2247E-16 RMS T 8.1605E-12 NORMALIZED 2.7882E-14 RMS Th 6.1777E-12 NORMALIZED 2.0591E-14 RMS geo 4.3877E-11 NORMALIZED 5.9630E-15 ############################################################################

ambrad · 2023-02-28T20:23:22Z

On Ascent, optimized and debug SCREAMv1 ERS tests pass with this PR merged in and this change made to EAMxx: E3SM-Project/scream#2198.

ambrad · 2023-02-28T22:14:44Z

With the fix to SyncUtils, homme_integration passes on Chrysalis except with the expected fails:

HOMMEBFB_P24.f19_g16_rx1.A.chrysalis_intel
    1 - verifyBaselineResults (Failed)
   37 - thetanh-moist-bubble (Failed) // diffs are due to bubble_init pressure_thickness commit
   38 - thetanh-dry-bubble (Failed)
   42 - preqx-nhgw-kokkos (Failed) // diff due to fixing the executable
   43 - preqx-nhgw-slice-kokkos (Failed)  // the rest are due to missing baselines
   45 - thetanh-moist-bubble-kokkos (Failed)
   46 - thetanh-dry-bubble-kokkos (Failed)
   47 - thetah-nhgw-kokkos (Failed)
   48 - thetanh-nhgw-kokkos (Failed)
   49 - thetah-nhgw-slice-kokkos (Failed)
   50 - thetanh-nhgw-slice-kokkos (Failed)
HOMME_P24.f19_g16_rx1.A.chrysalis_intel: both diffs are due to bubble_init pressure_thickness commit
   28 - thetanh-moist-bubble (Failed)
   29 - thetanh-dry-bubble (Failed)

It surprises me that more F90 nhgw tests don't diff against baselines because of fixing the executable used.

Next I'll run the usual integration testing and work on merging this PR.

ambrad · 2023-03-01T01:00:07Z

e3sm_developer on Chrysalis is as expected. I'll start integration tomorrow.

Hommexx: Add doubly-periodic capability. * Implement doubly-periodic mode in Hommexx. * Add C++-vs-F90 tests for the F90 nhgw and bubble tests. Fixes #4212. e3sm_developer and homme_integration pass except with the expected diffs and missing baselines in homme_integration. On Ascent, optimized and debug SCREAMv1 ERS tests pass with this PR merged in and this change made to EAMxx: E3SM-Project/scream#2198. [BFB] except for standalone Homme F90 bubble tests; adds new standalone Homme tests

ambrad added 4 commits January 31, 2023 20:13

Hommexx: Simplify sync util fns in one case.

198d77a

This also solves the multi-defs problem in the case of QSIZE_D == NUM_TIME_LEVELS.

Hommexx: Add double-periodic capability.

fb7e7c0

* Implement doubly-periodic mode in Hommexx. * Add C++-vs-F90 tests for the F90 nhgw and bubble tests.

Hommexx: Add a missing fence to the pre-CAAR w_i update loop.

d153f37

ambrad mentioned this pull request Feb 28, 2023

HOMMEXX-side doubly periodic capability E3SM-Project/scream#2118

Closed

ambrad added HOMME standalone issues with the standalone HOMME code that dont impact E3SM EAMxx PRs focused on capabilities for EAMxx enhancement labels Feb 28, 2023

ambrad requested review from oksanaguba and bartgol February 28, 2023 02:36

ambrad self-assigned this Feb 28, 2023

bartgol approved these changes Feb 28, 2023

View reviewed changes

oksanaguba reviewed Feb 28, 2023

View reviewed changes

oksanaguba mentioned this pull request Feb 28, 2023

homme planar preqx test using theta #4212

Closed

oksanaguba approved these changes Feb 28, 2023

View reviewed changes

ambrad mentioned this pull request Feb 28, 2023

EAMxx: Remove cosmetic use of rearth, anticipating its upstream removal. E3SM-Project/scream#2198

Merged

Hommexx: Fix dim issue in SyncUtils.

ad4175c

ambrad merged commit da29aea into E3SM-Project:master Mar 2, 2023

brhillman mentioned this pull request Mar 7, 2023

Add SCREAMv1 test to e3sm_gpucxx suite #5495

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hommexx: Add doubly-periodic capability. #5481

Hommexx: Add doubly-periodic capability. #5481

ambrad commented Feb 28, 2023 •

edited by oksanaguba

Loading

bartgol left a comment

oksanaguba Feb 28, 2023

ambrad Feb 28, 2023

oksanaguba Feb 28, 2023

ambrad Feb 28, 2023 •

edited

Loading

oksanaguba Feb 28, 2023

ambrad Feb 28, 2023

oksanaguba Feb 28, 2023

ambrad Feb 28, 2023

ambrad commented Feb 28, 2023

ambrad commented Feb 28, 2023 •

edited

Loading

ambrad commented Mar 1, 2023 •

edited

Loading

		@@ -283,6 +282,10 @@ subroutine bubble_init(elem,hybrid,hvcoord,nets,nete,f)

		hvcoord%hyai = ai; hvcoord%hybi = bi

		do k = 1,nlev

	dp(:,:,k) = ( hvcoord%hyai(k+1) - hvcoord%hyai(k) )*hvcoord%ps0 + &
	( hvcoord%hybi(k+1) - hvcoord%hybi(k) )*elem(ie)%state%ps_v(:,:,np1)

Hommexx: Add doubly-periodic capability. #5481

Hommexx: Add doubly-periodic capability. #5481

Conversation

ambrad commented Feb 28, 2023 • edited by oksanaguba Loading

bartgol left a comment

Choose a reason for hiding this comment

oksanaguba Feb 28, 2023

Choose a reason for hiding this comment

ambrad Feb 28, 2023

Choose a reason for hiding this comment

oksanaguba Feb 28, 2023

Choose a reason for hiding this comment

ambrad Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

oksanaguba Feb 28, 2023

Choose a reason for hiding this comment

ambrad Feb 28, 2023

Choose a reason for hiding this comment

oksanaguba Feb 28, 2023

Choose a reason for hiding this comment

ambrad Feb 28, 2023

Choose a reason for hiding this comment

ambrad commented Feb 28, 2023

ambrad commented Feb 28, 2023 • edited Loading

ambrad commented Mar 1, 2023 • edited Loading

ambrad commented Feb 28, 2023 •

edited by oksanaguba

Loading

ambrad Feb 28, 2023 •

edited

Loading

ambrad commented Feb 28, 2023 •

edited

Loading

ambrad commented Mar 1, 2023 •

edited

Loading