Skip to content

Tiled splineTransform! for RLR/SLR and multi-tile stride fix for 3D z-major layouts#23

Open
mmbell wants to merge 3 commits into
mainfrom
feature/rlr-tiling
Open

Tiled splineTransform! for RLR/SLR and multi-tile stride fix for 3D z-major layouts#23
mmbell wants to merge 3 commits into
mainfrom
feature/rlr-tiling

Conversation

@mmbell

@mmbell mmbell commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Distributed support for the 3D spline-vertical grids, driven by the Scythe moist_compressible_RLR equation set (radially tiled RLR runs are needed because the 3D cylindrical grids are computationally expensive):

  • Add the 3-argument (tiled) splineTransform! for RLR_Grid — it existed for RZ/RiRk, RL, SL, RLZ and SLZ but not RLR, so any distributed RLR run failed with a MethodError at the first spectral sync. Same per-z_b block layout as the RLZ method, with the spline vertical's b_kDim blocks.
  • Add the matching SLR_Grid method (same layout; the spherical spline-vertical grid had no tiled form either).
  • Fix the patch strides in the z-major tiled methods (RLZ/RLR/SLZ/SLR): the patch z-level base index was computed with the tile's wavenumber block count. Sub-blocks are aligned by wavenumber, but an inner tile carries fewer wavenumbers than the patch (its kDim follows its outermost ring), so on inner tiles every z level past the first read the wrong shared rows. Single-tile (tile = patch) behavior is unchanged — the strides coincide, so existing single-worker results are bit-identical.

Tests

  • New 3D multi-tile splineTransform! testset in test/tiling.jl: a B→A round trip over a 3-way radial decomposition for RLZ, RLR and SLR, seeded with wavenumber-1 content so the k ≥ 1 blocks are exercised. Verified to fail against the pre-fix stride math (the three value checks catch it) and pass with the fix.
  • Full suite: 35246/35246 pass.
  • End-to-end (Scythe side, companion commit on development): the same moist_compressible_RLR warm-bubble run with 1 worker vs 2 radial tiles for 300 steps agrees to ~4e-15 relative on every prognostic field.

mmbell added 2 commits July 15, 2026 17:29
The 3-argument (sharedSpectral, patch, tile) form existed for RZ/RiRk, RL,
SL, RLZ and SLZ but not RLR, so any distributed RLR run failed with a
MethodError at the first spectral sync. Same per-z_b block layout as the
RLZ method (k = 0 block, then real/imag pairs per azimuthal wavenumber),
with the spline vertical's b_kDim blocks.
…lti-tile tests

The 3-argument splineTransform! for the z-major 3D layouts (RLZ/SLZ, and the
just-added RLR) computed the PATCH z-level base index with the TILE's
wavenumber block count. Sub-blocks are aligned by wavenumber, but an inner
tile carries fewer wavenumbers than the patch (its kDim follows its outermost
ring), so every z level past the first read the wrong shared rows on inner
tiles. Single-tile (tile = patch) behavior is unchanged: the strides coincide.

Adds the missing SLR 3-argument method (same layout as RLR), and a
multi-tile B->A round-trip test for RLZ/RLR/SLR that seeds wavenumber-1
content and reconstructs the physical field on every tile of a 3-way radial
decomposition; verified to fail against the pre-fix stride math.
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.52941% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/tiling.jl 98.52% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/tiling.jl 84.87% <98.52%> (+0.42%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…l count

num_columns(::RRR_Grid) returned b_jDim * b_kDim — a spectral block count —
while every other grid with a vertical dimension returns the number of
physical z-fastest columns (iDim for RZ/RiRk, jDim for RLZ/RLR/SLZ/SLR).
Model drivers advance the state one vertical column at a time and stride by
kDim, so the RRR value must be iDim * jDim. No caller used the old value.
@mmbell

mmbell commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

Added b0f664c: num_columns(::RRR_Grid) returned the spectral count b_jDim * b_kDim instead of the physical vertical-column count iDim * jDim that every other vertical-column grid returns (iDim for RZ/RiRk, jDim for RLZ/RLR/SLZ/SLR). Model drivers stride the physical array by kDim over num_columns, so the old value under-covered the grid; no existing caller depended on it. Needed by the new Scythe moist_compressible_RRR set.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant