Skip to content

Rename direction-agnostic _rsx_ to _rs_ in m_riemann_solvers/m_rhs/m_viscous/m_surface_tension (NOT m_cbc) #1521

Description

@sbryngelson

In the Riemann/reconstruction modules, the reshaped working arrays carry an _rsx_ suffix — e.g. qL_prim_rsx_vf, qR_prim_rsx_vf, flux_rsx_vf, flux_src_rsx_vf, flux_gsrc_rsx_vf, vel_src_rsx_vf, mom_sp_rsx_vf, Re_avg_rsx_vf, qL_rsx_vf/qR_rsx_vf, dqL_rsx_vf/dqR_rsx_vf, vSrc_rsx_vf. The x reads as "x-direction," but these buffers hold the reshaped data for whichever split direction is currently being swept (the same array is reused for the x, y, and z sweeps via norm_dir/id). The x is a historical artifact and is actively misleading. The module's own doc comments already describe these states with the direction-agnostic _rs_ form ("…given in qK_prim_rs_vf … where ds = dx, dy or dz").

Proposed: rename _rsx__rs_ in the four files where it is direction-agnostic:

  • src/simulation/m_riemann_solvers.fpp (453 occurrences)
  • src/simulation/m_rhs.fpp (34)
  • src/simulation/m_viscous.fpp (4)
  • src/simulation/m_surface_tension.fpp (8)

⛔ Explicitly EXCLUDE m_cbc.fpp

src/simulation/m_cbc.fpp also contains rsx (24×), but there it is genuinely direction-specific: it is one leg of an rsx/rsy/rsz triple (q_prim_rsx_vf/rsy/rsz, F_rsx_vf/rsy/rsz, flux_rsx_vf_l/rsy/rsz), and the x/y/z is Fypp-generated via rs${XYZ}$ from #:for CBC_DIR, XYZ in [(1,'x'),(2,'y'),(3,'z')]. Renaming rsxrs there would collide with rsy/rsz and break the build. m_cbc's directional triple belongs to the struct-consolidation work in #1440, not here.

Why it's worth it: _rsx_ is the single biggest readability tax in m_riemann_solvers — a newcomer reading the y/z sweep arms sees "x" arrays everywhere. _rs_ is the right target: it matches an existing _rs_-family convention in the tree, and the #1426 refactor's own design sketch already writes qL_prim_rs_vf / build_mixture_state(...). So this rename is aligned with #1426, not opposed.

Pure rename, no behavior change: confirmed — rsx is never a string key, namelist var, index, or IO token (outside m_cbc's Fypp generation), and there are zero references in the Python toolchain, docs, tests, or case files. Golden files compare numeric output, so a name-only change won't move them.

Sequencing (important)

This module is under active refactoring (#1426 dedup; #1440 directional dispatch via #1432, currently draft). A ~500-line rename landing concurrently would cause merge pain in the most-edited file. Recommend either:

Either way, sequence it ahead of #1426's structural changes, not alongside.

Gotcha

GPU_DECLARE/@:ALLOCATE/@:DEALLOCATE lines carry these names (e.g. $:GPU_DECLARE(create='[flux_rsx_vf, flux_src_rsx_vf]')) and must be renamed in lockstep. A miss is a compile error, not a silent bug. Rebuild + run the full simulation suite across the CI compilers (golden files should not move).

Related: #1426, #1440.


Filed from a repo-wide code-cleanliness review; verified against master @ 40dde5e.

Code references

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions