Skip to content

Commit dd6269c

Browse files
committed
Refactor of vertical reconstruction with six new schemes
This refactor is motivated by i) adding a set of new reconstructions schemes that are ready-to-use-now for remapping, but ii) will be an essential part of a new grid generator that will use some of the new member methods available only as a result of the refactor. The "refactor" actually duplicates many existing schemes, leaving the original code in place to provide a period of transition just in case there's some major reason to revert to the old style of code. What changed ============ - added 16 modules name Recon1d_*.F90, which extend a base Recon1d_type.F90 - MOM_remapping.F90 - rewrote remapping_unit_tests to be tidier - added tests for 1) invariance when remapping to/from the same grid; 2) preservation of a uniform value (many of the old schemes fail); 3) internal consistency such as monotonic ordering of values at edges, no internal extrema, etc., but only for the new Recon1d_* schemes. - added more schemes to config_src/drivesr/timing_tests/time_MOM_remapping.F90 - added documentation via _Vertical_Reconstruction.dox with references Six new schemes are added: C_PLM_CW, C_PLM_CWK, C_MPLM_CWK, C_EMPLM_CWK, C_PPM_CWK, and C_EPPM_CWK. Ten existing schemes were ported to the new class, along with their non-monotonic behaviors or ill-formed expressions. Others can be ported later if needed, although one result of the refactor was the discovery that none of the existing schemes worked as desired. The new form uses a class (Recon1d) to define a uniform API for all the reconstruction schemes. The methods of the class include: - init() that creates work arrays and stores parameters such as `h_neglect` - reconstruct() that causes the reconstruction parameters to be calculated - average() that returns the average between two points in a reconstruction (used in the remap_src_to_sub_grid() functions) - unit_tests() method - check_reconstruction() checks that a reconstruction has the properties that it should (e.g. reconstruction values are monotonic) Theses properties are defined by each scheme, although many are shared between schemes. - f() that evaluates the reconstruction at a point (this will be used in the new grid-generator). - dfdx() that evaluates the derivative of the reconstruction at a point (this will be used in the new grid-generator) Testing ======= New tests have been added that check uniform states are preserved, states are preserved under unchanging grid, and that the reconstructions are internally consistent according to their own definition. This last is only available for the new class-based schemes via the check_reconstruction() method which can be customized to each reconstruction, which allows us to document which properties (e.g. monotonicity) are satisfied and which are not. These new tests use seeded random numbers which has proven useful for debugging/development, but are not necessarily useful in the CI. Nevertheless, I've made it part of remapping_unit_tests() so there is a template for debugging/testing new schemes when we add them. The same sequence of random numbers is used for each scheme, but the numbers are compiler dependent so the documented "fails" record the compiler version in comments. The helper type was previously broken out of MOM_remapping to make testing convenient. Performance =========== - the new class-based schemes are all faster than their counterparts in the original code style; - example numbers can be found in the build-test-perfmon job of this commit (expand the "Display timing results"); - e.g. timings for PPM_HYBGEN and C_PPM_HYBGEN show a marginal (~10%) speed up due to code style, and C_PPM_CWK has a more substantial speed up (~20%) due to a lighter algorithm; - I imagine we will see further speed ups if we worked on arrays rather than columns, which is an extension that could be implemented later. Documentation ============= - the new module APIs are documented; - a new page, `_Vertical_Reconstruction.dox`, details the vertical reconstruction schemes, tabulates the values of `REMPAPPING_SCHEME`, and summarize some properties. Minor (unrelated) ================= - removed `-j` for nested make in `.testing/Makefile`
1 parent de1ff7c commit dd6269c

25 files changed

+6251
-109
lines changed

.testing/Makefile

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,16 +267,16 @@ $(BUILD)/timing/Makefile: MOM_ACFLAGS += --with-driver=timing_tests
267267
# Build executables
268268
.NOTPARALLEL:$(foreach e,$(UNIT_EXECS),$(BUILD)/unit/$(e))
269269
$(BUILD)/unit/test_%: $(BUILD)/unit/Makefile FORCE
270-
cd $(@D) && $(TIME) $(MAKE) $(@F) -j
270+
cd $(@D) && $(TIME) $(MAKE) $(@F)
271271
$(BUILD)/unit/Makefile: $(foreach e,$(UNIT_EXECS),../config_src/drivers/unit_tests/$(e).F90)
272272

273273
.NOTPARALLEL:$(foreach e,$(TIMING_EXECS),$(BUILD)/timing/$(e))
274274
$(BUILD)/timing/time_%: $(BUILD)/timing/Makefile FORCE
275-
cd $(@D) && $(TIME) $(MAKE) $(@F) -j
275+
cd $(@D) && $(TIME) $(MAKE) $(@F)
276276
$(BUILD)/timing/Makefile: $(foreach e,$(TIMING_EXECS),../config_src/drivers/timing_tests/$(e).F90)
277277

278278
$(BUILD)/%/MOM6: $(BUILD)/%/Makefile FORCE
279-
cd $(@D) && $(TIME) $(MAKE) $(@F) -j
279+
cd $(@D) && $(TIME) $(MAKE) $(@F)
280280

281281
# Target codebase should use its own build system
282282
$(BUILD)/target/MOM6: $(BUILD)/target FORCE | $(TARGET_CODEBASE)

config_src/drivers/timing_tests/time_MOM_remapping.F90

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,30 @@ program time_MOM_remapping
99
implicit none
1010

1111
type(remapping_CS) :: CS
12-
integer, parameter :: nk=75, nij=20*20, nits=10, nsamp=100, nschemes = 2
13-
character(len=10) :: scheme_labels(nschemes)
12+
integer, parameter :: nk=75, nij=20*20, nits=10, nsamp=100, nschemes = 22
13+
character(len=16) :: scheme_labels(nschemes) = [ character(len=16) :: &
14+
'PCM', &
15+
'C_PCM', &
16+
'PLM', &
17+
'C_MPLM_WA', &
18+
'C_EMPLM_WA', &
19+
'C_PLM_HYBGEN', &
20+
'C_PLM_CW', &
21+
'C_PLM_CWK', &
22+
'C_MPLM_WA_POLY', &
23+
'C_EMPLM_WA_POLY', &
24+
'C_MPLM_CWK', &
25+
'PPM_H4', &
26+
'PPM_IH4', &
27+
'PQM_IH4IH3', &
28+
'PPM_CW', &
29+
'PPM_HYBGEN', &
30+
'C_PPM_H4_2018', &
31+
'C_PPM_H4_2019', &
32+
'C_PPM_HYBGEN', &
33+
'C_PPM_CW', &
34+
'C_PPM_CWK', &
35+
'C_EPPM_CWK' ]
1436
real, dimension(nschemes) :: timings ! Time for nits of nij calls for each scheme [s]
1537
real, dimension(nschemes) :: tmean ! Mean time for a call [s]
1638
real, dimension(nschemes) :: tstd ! Standard deviation of time for a call [s]
@@ -31,9 +53,6 @@ program time_MOM_remapping
3153
seed(:) = 102030405
3254
call random_seed(put=seed)
3355

34-
scheme_labels(1) = 'PCM'
35-
scheme_labels(2) = 'PLM'
36-
3756
! Set up some test data (note: using k,i indexing rather than i,k)
3857
allocate( u0(nk,nij), h0(nk,nij), u1(nk,nij), h1(nk,nij) )
3958
call random_number(u0) ! In range 0-1
@@ -61,8 +80,8 @@ program time_MOM_remapping
6180
do isamp = 1, nsamp
6281
! Time reconstruction + remapping
6382
do ischeme = 1, nschemes
64-
call initialize_remapping(CS, remapping_scheme=trim(scheme_labels(ischeme)), &
65-
h_neglect=h_neglect, h_neglect_edge=h_neglect)
83+
call initialize_remapping(CS, remapping_scheme=trim(scheme_labels(ischeme)), nk=nk, &
84+
h_neglect=h_neglect, h_neglect_edge=h_neglect)
6685
call cpu_time(start)
6786
do iter = 1, nits ! Make many passes to reduce sampling error
6887
do ij = 1, nij ! Calling nij times to make similar to cost in MOM_ALE()

config_src/drivers/unit_tests/test_MOM_remapping.F90

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,18 @@ program test_MOM_remapping
22

33
use MOM_remapping, only : remapping_unit_tests
44

5-
if (remapping_unit_tests(.true.)) stop 1
5+
integer :: n !< Number of arguments, or tests
6+
character(len=12) :: cmd_ln_arg !< Command line argument (if any)
7+
8+
n = command_argument_count()
9+
10+
if (n==1) then
11+
call get_command_argument(1, cmd_ln_arg)
12+
read(cmd_ln_arg,*) n
13+
else
14+
n = 3000 ! Fallback value if no argument provided
15+
endif
16+
17+
if (remapping_unit_tests(.true., num_comp_samp=n)) stop 1
618

719
end program test_MOM_remapping

docs/discrete_space.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,5 @@ algorithm.
1717
api/generated/pages/Discrete_Coriolis
1818
api/generated/pages/Discrete_PG
1919
api/generated/pages/Energetic_Consistency
20+
api/generated/pages/Vertical_Reconstruction
2021
api/generated/pages/Discrete_OBC

docs/zotero.bib

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2946,3 +2946,17 @@ @article{Young1994
29462946
pages={1812--1826},
29472947
year={1994}
29482948
}
2949+
2950+
@article{van_leer_1977,
2951+
title = {Towards the ultimate conservative difference scheme. {IV}. {A} new approach to numerical convection},
2952+
volume = {23},
2953+
issn = {0021-9991},
2954+
doi = {10.1016/0021-9991(77)90095-X},
2955+
number = {3},
2956+
journal = {Journal of Computational Physics},
2957+
author = {Van Leer, Bram},
2958+
month = mar,
2959+
year = {1977},
2960+
pages = {276--299},
2961+
}
2962+

src/ALE/MOM_ALE.F90

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,15 +257,15 @@ subroutine ALE_init( param_file, GV, US, max_depth, CS)
257257
h_neglect = GV%kg_m2_to_H * 1.0e-30 ; h_neglect_edge = GV%kg_m2_to_H * 1.0e-10
258258
endif
259259

260-
call initialize_remapping( CS%remapCS, string, &
260+
call initialize_remapping( CS%remapCS, string, nk=GV%ke, &
261261
boundary_extrapolation=init_boundary_extrap, &
262262
check_reconstruction=check_reconstruction, &
263263
check_remapping=check_remapping, &
264264
force_bounds_in_subcell=force_bounds_in_subcell, &
265265
om4_remap_via_sub_cells=om4_remap_via_sub_cells, &
266266
answer_date=CS%answer_date, &
267267
h_neglect=h_neglect, h_neglect_edge=h_neglect_edge)
268-
call initialize_remapping( CS%vel_remapCS, vel_string, &
268+
call initialize_remapping( CS%vel_remapCS, vel_string, nk=GV%ke, &
269269
boundary_extrapolation=init_boundary_extrap, &
270270
check_reconstruction=check_reconstruction, &
271271
check_remapping=check_remapping, &

0 commit comments

Comments
 (0)