Refactor of vertical reconstruction adding six new schemes #741
+6,449
−953
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
edges, no internal extrema, etc., but only for the new Recon1d_* schemes.
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:
h_neglect
(used in the remap_src_to_sub_grid() functions)
properties that it should (e.g. reconstruction values are monotonic)
Theses properties are defined by each scheme, although many are shared
between schemes.
new grid-generator).
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 base Recon1d class also provides a second type to make testing convenient.
In time, this could (should) be moved out to a stand-alone module, so long as
it does not become cumbersome. Currently this type is hidden to avoid expanding
the network of dependencies, particularly using anything in the
framework/
directory which is not needed by the class.
Performance
original code style;
(expand the "Display timing results" at https://github.com/adcroft/MOM6/actions/runs/11481161473/job/31951185232);
up due to code style, and C_PPM_CWK has a more substantial speed up (~20%)
due to a lighter algorithm;
columns, which is an extension that could be implemented later.
Documentation
_Vertical_Reconstruction.dox
, details the verticalreconstruction schemes, tabulates the values of
REMPAPPING_SCHEME
,and summarize some properties.
Minor (unrelated)
-j
for nested make in.testing/Makefile