E3SM icon indicating copy to clipboard operation
E3SM copied to clipboard

Evaluate time-dependent tendencies at correct time for each RK4 stage

Open sbrus89 opened this issue 1 year ago • 1 comments

This PR corrects a longstanding issue with RK4 and tendency terms which have an explicit time dependence (see https://github.com/E3SM-Project/E3SM/issues/5364 and https://github.com/MPAS-Dev/MPAS-Model/issues/375). Big thanks to @gcapodag and @jeremy-lilly for providing the fix in their LTS development branch linked in https://github.com/E3SM-Project/E3SM/issues/5364.

Here, I have migrated these changes from their MPAS-Dev branch to E3SM and added the modifications to correct the convergence issues for the manufactured solution test case: https://github.com/E3SM-Project/polaris/pull/72

[BFB] (for all standard E3SM tests)

sbrus89 avatar Oct 17 '24 18:10 sbrus89

Testing:

With this PR, the manufactured solution test case in Polaris converges at 2nd order when refining in space/time: image

When refining in time only, 4th order convergence is achieved: image

However, I have not been able to reproduce the 4th order convergence for the hurricane test case from Lilly et al. 2023 (Figure 4). The best I have been able to achieve is first order: convergence For this result, I ran from 2012-10-13_00:00:00 to 2012-10-13_01:00:00 starting from a 2012-10-13_00:00:00 restart file. I used dt=0.1 as a reference solution.

sbrus89 avatar Oct 17 '24 18:10 sbrus89

Relevant discussion of this PR can be seen in https://github.com/E3SM-Ocean-Discussion/E3SM/pull/55

xylar avatar Oct 28 '24 16:10 xylar

Tested with gnu on chicoma for both debug and optimized, compared to the master branch point, for MPAS-Ocean standalone and the compass nightly suite. I am able to get a bfb match between this branch and master. In particular, the RK4 tests are BFB:

00:02 PASS ocean_global_ocean_Icos240_WOA23_RK4_performance_test
00:07 PASS ocean_global_ocean_Icos240_WOA23_RK4_restart_test
00:05 PASS ocean_global_ocean_Icos240_WOA23_RK4_decomp_test
00:05 PASS ocean_global_ocean_Icos240_WOA23_RK4_threads_test

That makes sense, because config_use_time_varying_atmospheric_forcing and config_use_time_varying_land_ice_forcing are both false for these tests, so the PR should change nothing.

One small note: The step ocean_global_ocean_Icos240_WOA23_init didn't match between this PR and master, which seems unrelated to this PR. But then I reran it and it matched. That is a mystery, but maybe I just made a mistake in my run sequence.

I also tested E3SM on perlmutter. This does not exercise the RK4 code, but it at least compiles it. E3SM passes:

./create_test SMS_Ld3.T62_oQU240.GMPAS-NYF.pm-cpu_intel -p m4572 -q debug --walltime 00:30:00
./create_test SMS_Ld3.T62_oQU240.GMPAS-NYF.pm-cpu_gnu  -p m4572 -q debug --walltime 00:30:00

mark-petersen avatar Oct 29 '24 14:10 mark-petersen

@mark-petersen, how are you able to do standalone testing on Chicoma? There shouldn't be compatible spack packages until https://github.com/MPAS-Dev/compass/pull/865 goes in.

xylar avatar Oct 29 '24 14:10 xylar

Hmm... I'm on the head of compass/main and created the load_dev* this morning, then compiled and it ran fine. Here are my steps:

git log -n 1
commit cab53e2c8dc9be0efbd4a7a6c2150b0c599c1870 (HEAD -> main, origin/main, origin/HEAD)
Merge: 0e79f61e3 8b35df7f3
Author: Xylar Asay-Davis <[email protected]>
Date:   Sat Oct 19 11:04:22 2024 -0600
    Merge pull request #836 from xylar/add-normalized-dib-dismf

./conda/configure_compass_env.py --conda ${HOMEDIR}/miniforge3 --compiler gnu
source load_dev_compass_1.5.0-alpha.1_chicoma-cpu_gnu_mpich.sh
compass suite -s -c ocean -t nightly -p $e/master/components/mpas-ocean -w $n/$DIR

etc. And I had compiled stand-alone in separate directories, not in the compass submodule. It all worked fine, so I don't know.

mark-petersen avatar Oct 29 '24 14:10 mark-petersen

I see. I'm a little surprised that works. If you were to try to recreate the load script, I don't think it would work -- at least that's what others have reported.

xylar avatar Oct 29 '24 15:10 xylar

I think the first two plots above show that this PR is working as intended. Those manufactured solutions provide the forcing at a requested moment in time. The hurricane test case (third plot) shows 1st rather than expected 4th order convergence. That was tricky to obtain in Lilly et al. 2023, and there must be some small difference in the way that the forcing data is interpolated in time. I'm happy for this PR to be merged despite that difference. The manufactured solution shows that this code in this PR is correct. In practice the wind data is updated at 1 or 6 hour intervals, so the model does not need interpolation that satisfies fourth-order convergence at very small time intervals.

mark-petersen avatar Oct 29 '24 18:10 mark-petersen

@cbegeman @xylar any updates?

rljacob avatar Nov 21 '24 18:11 rljacob

Passes:

  • SMS_D_Ld1.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel.allactive-wcprod

merged to next

jonbob avatar Dec 02 '24 22:12 jonbob

merged to master

jonbob avatar Dec 03 '24 17:12 jonbob