Three independent clusters of unexplained magic literals.
1. Hardcoded downsample factor 3 — int((dim + 1)/3) - 1 with no named constant:
src/simulation/m_start_up.fpp:299-305 (m_ds, n_ds, p_ds, and *_glb_ds).
src/simulation/m_data_output.fpp:1657-1659 (m_ds/n_ds/p_ds; the down_sample block also spans L662-740).
These duplicate the same hardcoded /3 in two files; should be a single named constant (e.g. down_sample_factor = 3).
2. Repeated 1e-300 log floor — toolchain/mfc/viz/interactive.py clamps before log10 in ~8 places (L302, 303, 304, 380, 382, 383, 1854, 1856, 1857; plus related 1e-30 scale floors at L152, 1936, 2369). A single module-level constant (e.g. LOG_FLOOR = 1e-300) would dedupe and document intent.
3. s_perturb_primitive physics-coupled magic — src/pre_process/m_assign_variables.fpp:
- L145:
p0 = 101325._wp (atmospheric pressure in Pa — undocumented literal).
- L149:
if (j < 177) — couples a physics perturbation to a hardcoded x-index 177, i.e. an undocumented mesh-resolution assumption. This silently does the wrong thing on any grid where index 177 isn't the intended physical location.
Why it's friction: all three are "what does this number mean?" hazards. The j < 177 one is the most dangerous: it bakes a grid assumption into physics with no comment.
Important — do NOT change values: items 1 and 3 feed golden-file-gated output. The fix is to name and comment (p0 → a documented standard-pressure constant; 177 → a named/commented index, ideally derived from grid params), not to alter the numbers.
Filed from a repo-wide code-cleanliness review; verified against master @ 40dde5e.
Code references
Three independent clusters of unexplained magic literals.
1. Hardcoded downsample factor
3—int((dim + 1)/3) - 1with no named constant:src/simulation/m_start_up.fpp:299-305(m_ds,n_ds,p_ds, and*_glb_ds).src/simulation/m_data_output.fpp:1657-1659(m_ds/n_ds/p_ds; thedown_sampleblock also spans L662-740).These duplicate the same hardcoded
/3in two files; should be a single named constant (e.g.down_sample_factor = 3).2. Repeated
1e-300log floor —toolchain/mfc/viz/interactive.pyclamps beforelog10in ~8 places (L302, 303, 304, 380, 382, 383, 1854, 1856, 1857; plus related1e-30scale floors at L152, 1936, 2369). A single module-level constant (e.g.LOG_FLOOR = 1e-300) would dedupe and document intent.3.
s_perturb_primitivephysics-coupled magic —src/pre_process/m_assign_variables.fpp:p0 = 101325._wp(atmospheric pressure in Pa — undocumented literal).if (j < 177)— couples a physics perturbation to a hardcoded x-index177, i.e. an undocumented mesh-resolution assumption. This silently does the wrong thing on any grid where index 177 isn't the intended physical location.Why it's friction: all three are "what does this number mean?" hazards. The
j < 177one is the most dangerous: it bakes a grid assumption into physics with no comment.Important — do NOT change values: items 1 and 3 feed golden-file-gated output. The fix is to name and comment (
p0→ a documented standard-pressure constant;177→ a named/commented index, ideally derived from grid params), not to alter the numbers.Filed from a repo-wide code-cleanliness review; verified against
master@40dde5e.Code references
src/simulation/m_start_up.fpp:299-305— downsample /3src/simulation/m_data_output.fpp:1657-1659— downsample /3toolchain/mfc/viz/interactive.py:302-304— 1e-300 log floorsrc/pre_process/m_assign_variables.fpp:145-149— p0=101325, j<177