Repository navigation
Add sponge-layer tensor viscosity to theta-l_kokkos and EAMxx - #8642
Conversation
add a second set of tensor laplace coefficieints for use in the sponge layer and controlled by laplace_scaling
Complete the tensor Laplace sponge-layer port to EAMxx by wiring the laplace_scaling namelist variable through the full read/query path. Add laplace_scaling to the ctl_nl namelist declaration in namelist_mod.F90 so readnl() can parse it from atm_in instead of aborting; add a default entry (0.0, off) to namelist_defaults_eamxx.xml so EAMxx cases generate a value for it; and expose it via get_homme_real_param_f90 in homme_params_mod.F90 alongside hypervis_scaling so atmchange/atmquery can read it. Also flush iulog at the end of prim_printstate so periodic state diagnostics are written promptly rather than buffered. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extend preqx_kokkos's init_elements_2d_c (preqx_f2c_mod.F90, cxx_f90_interface_preqx.cpp, prim_driver_mod.F90) to copy and pass elem%tensorVisc_2 alongside tensorVisc, so ElementsGeometry::set_elem_data receives real data instead of a null default tensorvisc2 pointer. preqx_kokkos does not use tensorVisc_2 at runtime but now initializes uniformly with theta-l_kokkos. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dirk_ut.cpp: two Elements::init()/ef90.init() call sites still used the pre-sponge2 4-arg signature (with a stale consthv bool), leaving alloc_gradphis effectively false and gradphis-related buffers unallocated, causing a SIGSEGV during dirk_toplevel_testing. gllfvremap_ut.cpp: dstrain's buffer was sized using nf2 (FV-grid size) but is also viewed as a GLL-side array requiring np2 (NP*NP) elements. For nf < NP this under-allocated the buffer, causing an out-of-bounds access that manifests as a SIGSEGV in release builds (asserts stripped) and as an assertion failure in DEBUG builds (reported independently by Noel Keen). Fixed by sizing the buffer to max(nf2, np2). Both fixes validated via ./create_test -c -b master homme_integration (HOMME_P24 and HOMMEBFB_P24), 46/46 and 177/177 unit tests passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two changes needed together for the standalone HOMME driver: - cxx_f90_interface_theta.cpp: create HyperviscosityFunctor with (num_elems, params), matching the CaarFunctor pattern just above it. This forces the lazy-construction path (is_setup=false), so the existing setup_needed()/setup() call actually runs. Previously the no-args constructor set is_setup=true immediately, silently skipping setup() (and thus the copy of nu_scale_top/nu_scale_top_ilev_pack_lim from the Fortran ref states) whenever tom_sponge_start>0. - prim_driver_mod.F90 (prim_init2): call prim_init_ref_states_views before prim_init_kokkos_functors, so the ref states setup() reads are already populated. EAMxx's driver already has this ordering; standalone HOMME did not. The old, now-redundant call to prim_init_ref_states_views inside prim_init_elements_views is removed. Together these fix an uninitialized (rank-dependent garbage) m_nu_scale_top_ilev_pack_lim, which could cause crashes (e.g. asserts in BoundaryExchange registration) in standalone HOMME runs with tom_sponge_start>0. No change to HyperviscosityFunctorImpl.cpp: the existing setup()-based copy logic is reused as-is. Verified via a full homme_integration test suite run (--compare against master): both HOMMEBFB_P24 (177/177) and HOMME_P24 (46/46) passed, including BFB comparison. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bartgol
left a comment
There was a problem hiding this comment.
Would it be possible to give the new tensor variable a more descriptive name? tensorvisc_2 is not very self-explanatory. I know you explain what it is right next to where it is declared, but it is used in a few places around the code.
Perhaps something like sponge_layer_tensorvisc? Or something along that line? I know it's longer, but I think it would help self-documenting the code.
|
we have a constant coefficient laplace operator, and a tensor coefficient laplace operator, used for adding viscosity. We use this operator for several things - sponge layer and topo smoothing and hyperviscosity. So it would be a mistake to add sponge layer to the name of this variable. The current name is conflating the laplace operator with viscosity, so I agree that it could have a better name - maybe something like laplace_tensor. But then we should rename both, the tensor usually initialized with the dx^3 scalings (used by HV) and the new tensor initialized with dx^1 scalings (used by sponge layer). Topo smoothing also uses one of these tensors, with a dx^2 scaling. this rename would touch a lot of code, including a lot of files not modified by this PR, so lets do it as a separate, variable-rename only PR. |
|
Thanks for explaining why sponge layer would be inappropriate. We can definitely do this later. I just don't want this to become just another variable like |
|
@mt5555 I'm assuming the NL DIFFs are expected? I'm re running the oneapi testing since it had a mysterious build fail. Shouldn't be related to this PR, but I want to make sure |
|
Side note: the oneapi mysterious build fails are usually NOT failures. It's just that the output of |
yes - there's a new namelist variable, laplace_scaling |
Add a sponge-layer tensor viscosity option to the HOMME dycore, with EAM and EAMxx supporting namelist plumbing. Fixes bugs in how fortran initialization data is copiied over to C++/kokkos in theta-l_kokkos dycore. Also includes two unrelated unit-test fixes discovered during testing. [BFB] stealth feature [NLDIFFs] for EAMxx cases
|
Merged to next |
Add a sponge-layer tensor viscosity option to the HOMME dycore, with EAM and EAMxx supporting namelist plumbing. Fixes bugs in how fortran initialization data is copiied over to C++/kokkos in theta-l_kokkos dycore.
Also includes two unrelated unit-test fixes discovered during testing.
[BFB] stealth feature
some NLFAILs due to new laplace_scaling namelist variable
PR #8646 will modify tests to ensure this new feature is tested
Summary
This PR ports a tensor-viscosity-based sponge layer (top-of-model damping) from the Fortran preqx dycore to theta-l, theta-l_kokkos (C++/Kokkos), EAM and EAMxx. It introduces a second tensor Laplacian coefficient set (
tensorVisc_2), selectable via a newlaplace_scalingnamelist parameter, and wires it through the full Fortran/C++ interop stack.Changes
tensor laplace option: Add a second set of tensor Laplacian coefficients (tensorVisc_2) for use in the sponge layer, controlled by the newlaplace_scalingnamelist parameter.also DSS and linearlize the new tensorVisc_2: Complete the Fortran-side DSS (direct-stiffness-summation) and linearization support fortensorVisc_2.port tensor Laplace operator to EAMxx: Port the tensor Laplacian operator support to the C++SphereOperators/HyperviscosityFunctorImplused by theta-l_kokkos/EAMxx.fix(eamxx): expose laplace_scaling in ctl_nl namelist for EAMxx: Wirelaplace_scalingthrough the full read/query path — add it to thectl_nlnamelist declaration, add a default (off) entry tonamelist_defaults_eamxx.xml, and expose it viaget_homme_real_param_f90alongsidehypervis_scaling. Also flushiulogat the end ofprim_printstatefor prompter diagnostic output.add ability to set new laplace_scalings in EAM: Add the EAM-side namelist definition entry forlaplace_scaling.fix(homme): repair unit test SIGSEGVs in dirk_ut and gllfvremap_ut:dirk_ut.cpp: twoElements::init()call sites needed update for new interface.gllfvremap_ut.cpp: unrelated buffer size error uncovered during testing.dstrain's buffer was sized usingnf2(FV-grid size) but is also viewed as a GLL-side array requiringnp2elements, under-allocating fornf < NP. Fixed by sizing tomax(nf2, np2).fix(homme): init nu_scale_top_ilev_pack_lim for tom_sponge_start>0: Fix issues with how theta-l_kokkos gets data from the Fortran init functions ( following what was done in a previous fix for EAMxx ) ensuringprim_init_ref_states_viewsis called in the proper sequence.Testing
HOMMEBFB suite on pm-gpu verifies fortran vs c++ remains BFB