-
Notifications
You must be signed in to change notification settings - Fork 3
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
Implement Runge-Kutta time integration #86
Implement Runge-Kutta time integration #86
Conversation
@matthewhoffman, I expect that this will require significant refactoring for flexibility, but I think having the in-progress PR open now will be useful for discussion and testing. Note that only the last six commits here are really relevant, as the others are due to merging my FCT branch into this one. I can clean up that history once the FCT branch is merged into |
This comment was marked as outdated.
This comment was marked as outdated.
b882525
to
bb27e54
Compare
709919e
to
4f69cfe
Compare
One outstanding issue: what do we do about calculating It seems like this should be moved to occur after the RK loop. This is also a first-order treatment. Is it possible to update it to higher order when using fct advection? |
TestingI ran the
We get close-to-third-order convergence for third order advection with three-stage RK3: Using the same settings but with Using second-order advection and RK2:
|
Results of slotted cylinder test case using |
Updating this to higher order in #94. Once that is merged I will rebase this branch and try to calculate the final |
Testing the calculation of Forward Euler (6d0d572): Forward Euler (3ec981a): RK3 (3ec981a): fractional error is calculated as |
Here are Forward Euler: all tests pass execution and validation. I have not yet run a baseline comparison.
3-stage RK3: All tests pass execution, one fails validation:
4-stage RK3: All tests pass execution, one fails validation:
|
A few data points on the
|
The small differences for the |
@trhille , the level of documentation you've made in this PR is really great. A few items for you:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trhille , as monumental as this PR is, the implementation seems fairly straightforward, and I only have minor comments/requests. The big work is probably in evaluating and blessing the changes to all the compass test cases. Have you looked at baseline comparisons for Forward Euler yet? I wonder how gruesome it is. Given this PR is primarily bringing in new functionality that is not currently in production runs, and you've demonstrated that functionality is working as expected, at least to our current expectations, if the changes from baseline tests with FE make sense and are palatable, I don't see any obstructions to getting this merged in the next week or so.
Regarding the hydro_radial restart weirdness, I'm not entirely opposed to merging this without resolving that, but maybe after having looked this over carefully I'll have other ideas about what to look into.
components/mpas-albany-landice/src/mode_forward/mpas_li_time_integration_fe_rk.F
Show resolved
Hide resolved
components/mpas-albany-landice/src/mode_forward/mpas_li_time_integration_fe_rk.F
Outdated
Show resolved
Hide resolved
components/mpas-albany-landice/src/mode_forward/mpas_li_time_integration_fe_rk.F
Outdated
Show resolved
Hide resolved
components/mpas-albany-landice/src/mode_forward/mpas_li_time_integration_fe_rk.F
Show resolved
Hide resolved
components/mpas-albany-landice/src/mode_forward/mpas_li_time_integration_fe_rk.F
Show resolved
Hide resolved
components/mpas-albany-landice/src/mode_forward/mpas_li_time_integration_fe_rk.F
Show resolved
Hide resolved
components/mpas-albany-landice/src/mode_forward/mpas_li_time_integration_fe_rk.F
Outdated
Show resolved
Hide resolved
components/mpas-albany-landice/src/mode_forward/mpas_li_time_integration_fe_rk.F
Outdated
Show resolved
Hide resolved
components/mpas-albany-landice/src/mode_forward/mpas_li_time_integration_fe_rk.F
Outdated
Show resolved
Hide resolved
Thanks for the review, @matthewhoffman! I'm glad it was more straightforward to review than we probably both anticipated.
It's definitely tempting to merge without fixing the hydro_radial restart issue, but it also makes me nervous because then we have a baseline that we know to be incorrect for that test. |
@trhille , do you have any results on relative computational cost between these schemes? (e.g. compass timers) I don't want a whole performance study for this PR, but a rough sense of the impact would be good to know. (Though that may change depending on how we handle the 'extra' calving velocity solve issue.) |
Compass runtimes on Chicoma: Note that |
@matthewhoffman, sorry I somehow missed this question. This is a good point. We could include a call to Update: I've move the call to |
ada4117
to
68cdce6
Compare
Add RK2 loop in time integration module. This is the bare bones of RK2 time stepping. Compiles but likely will not run.
Correct a description in Registry, update comments and log writes.
Fix bug in groundedSfcMassBalApplied after Runge-Kutta loop. It was inadvertently being assigned the enitre applied SMB field instead of just the grounded portion.
Calculate the total groundedToFloatingThickness and fluxAcrossGroundingLine at the end of the RK loop. Budgets do not yet close, but are reasonably close.
Reset layerThickness when using config_restore_thickness_after_advection. Restoring thickness alone is not sufficient when using Runge-Kutta time integration because the advected layerThickness gets propagated through all stages of the Runge-Kutta loop, which leads to advection and surface/basal mass balances causing thickness change.
Update stresses and strain rates after calving. This allows restart test with damage to pass validation, but may not be the most rigorous fix.
Update thickness and tracer halos before velocity solve. This allows eismint2 decomposition test to pass validation.
Update temperature and waterFrac in RK loop, but not enthalpy. Updating enthalpy resulted in negative temperatures, which caused an error in li_thermal. Updating temperature and waterFrac instead after enthalpy is advected allows enthalpy tests in full_integration suite to pass execution and validation.
Simple clean-up after code review, including moving call to mask calculation, initializing waterFracPrev to zeroes, adding return statements after error messages. Also add a halo update on cellMask and edgeMask before calculating groundedToFloatingThickness and fluxAcrossGroundingLineOnCells.
Initialize the various RK weights (rkSSPweights, rkTendWeights, rkSubstepWeights) to large negative values which are overwritten for each case. This will make it obvious if a weight is being used when it should not be.
Restore calving front within RK loop to prevent velocity solutions on extended ice geometry.
Remove support for config_rk_order = 1 to avoid unnecessary redundancy with config_time_integration = forward_euler while maintaining backward compatibility.
Include calving in Runge-Kutta loop, rather than implementing it afterwards.
Calculate layerThickness after calving in RK loop, which is necessary when calculating weighted averages for RK integration.
This reverts commit 5e23b98.
This reverts commit 813ee05. After testing and discussion, it is apparent that including calving within the Runge-Kutta loop is not desirable because it changes the domain between stages of the Runge-Kutta integration.
Calculate fluxAcrossGroundingLineOnCells in advection module for each RK stage and then take weighted average after RK integration instead of calcuating fluxAcrossGroundingLineOnCells on final state. This is more consistent with how fluxAcrossGroundingLine (on edges) is treated.
65f31e6
to
51802e1
Compare
…alving Add logic to control whether velocity is solved before and/or after calving. This allows both for backward compatibility and for the abilility to use self-consistent velocty, stress, and strain-rate fields when calculating calving.
Make config_update_velocity_before_calving = .false. the default. This is not necessarily the desired behavior for production runs, but it is necessary for baseline comparison when using forward Euler time integration.
…ving. Always update velocity after calving if it was not updated before calving.
… positions Move subglacial hydro and bed topography updates back to their original positions. These were initially moved due to the complexity of interaction with RK integration, but I think subsequent changes have made that move unnecessary.
Testing mesh convergence in a simplified test case with calving at e2a2f0b. I used the I then ran these forward for 10 years on one processor using the following relevant namelist settings, designed to result in a shelf of 30 km radius at the end of the run:
I then created the 30km-radius shelves at the same resolutions and used these as the "true" solution for each run. I adapted the analysis script from the |
Regression Testing
3-stage RK3: Run time on Chicoma = 20:00
4-stage RK3: Run time on Chicoma = 21:07
All tests using Runge-Kutta time integration fail baseline comparison except for This includes failure of baseline comparison for the I also tested Upon further testing, it turns out that I can get the hydro restart tests to pass validation by adding a call to |
Mesh Convergence TestingTesting mesh convergence using Matt's COMPASS branch at matthewhoffman/compass@080d1f0, using 1, 2 ,4, and 8km meshes. I used the following relevant namelist settings:
Meanwhile, the Halfar dome test gives first order convergence. It's unclear if that is because of the SIA velocity, the first-order treatment at the ice margins, or something else. Running for 1000 years instead of 200 years did not improve convergence order. |
Even more testingI ran the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trhille , this is amazing work! And the testing... I can't find anything to nitpick about and will merge this now. As we discussed, there are some subtleties with the RK treatment we don't fully understand, but all the meaningful configurations we want to support look great and the existing Forward Euler behavior is bit-for-bit. Thanks for documenting all the testing results.
This merge adds Runge-Kutta time integration to MALI, which is likely necessary when using higher-order advection. Currently, Runge-Kutta integration is used for damage evolution and thickness and tracer advection. Surface and basal mass balances are applied within the Runge-Kutta loop and budgets are calculated at the end of the loop. Front ablation (i.e., calving and face-melting) and bed topography updates are applied via forward Euler outside the Runge-Kutta loop. This could conceivably be updated for full consistency.
The Runge-Kutta formulations used here are the two-stage second-order and and three-stage third-order strong stability preserving schemes of Shu and Osher (1988), as well as the four-stage third-order strong stability preserving scheme of Spiteri and Ruuth (2002). See equations 2.47–2.49 in Durran (2010) for an overview.
There is one velocity solve per Runge-Kutta stage and an optional solve before calving (more on that below), meaning that the four-stage RK3 scheme is ~25–33% more expensive than the three-stage RK3 scheme for a given time step length. However, the four-stage scheme theoretically allows for an effective CFL fraction of 2.0, while the three-stage scheme is limited to CFL fraction of 1.0. Testing indicates stable results using the four-stage scheme for an effective CFL fraction of 1.8 for a simple grounded slab advecting at uniform speed. Note that when using the four-stage scheme in MALI, the maximum allowable time step is updated, so
config_adaptive_timestep_CFL_fraction
should still be between 0.0 and 1.0.There is now an optional extra velocity solve between advection and calving in addition to the final velocity solve at the end of the time step, for any choice of time integration scheme. The extra velocity solution prior to calving ensures a self-consistent set of inputs to calving routines; i.e., the velocity, stress, and strain rate fields will be consistent with the geometry used for calving, which is not the case when velocity is solved only after calving. This makes calving more accurate, but is obviously significantly more expensive. The extra velocity solution is enabled using
config_update_velocity_before_calving = .true.
. It is disabled by default to allow regression tests to pass baseline comparison. Note that the extra velocity solution is not necessary for some calving laws, such as damage threshold calving, so the user should carefully consider this option when setting up simulations. There is currently no check in the code to ensure the combination of calving and extra velocity solve settings are sensible. Also of note is thatli_restore_calving_front
(if enabled) is called before the extra velocity solution to prevent solving velocities on an extended geometry, while the remaining calving routines are called after the velocity solve. If the volume of dynamic ice is not changed by calving, then the final velocity solution is automatically skipped.References:
Durran, Dale R. Numerical Methods for Fluid Dynamics. Vol. 32. Texts in Applied Mathematics. New York, NY: Springer New York, 2010. https://doi.org/10.1007/978-1-4419-6412-0.
Shu CW, Osher S (1988) Efficient implementation of essentially nonoscillatory shock-capturing schemes. J Comp Phys 77:439–471
Spiteri RJ, Ruuth SJ (2002) A new class of optimal high-order strong-stability-preserving time discretization methods. SIAM J Numer Anal 40:469–491