s_save_data in src/post_process/m_start_up.fpp is a ~580-line routine (lines 169–747) that handles every post-processed output variable inline, plus two self-contained physics blocks that can be lifted into helpers to shrink the driver and isolate the machinery.
In scope: lift the bubble-moment block
Lines 683–724 (the if (bubbles_euler) moment block: the nR/nV/nP/nM loops over nb plus the adv_n number-density write) are a contiguous, self-contained unit sitting near the end of the routine.
→ extract s_write_bubble_moments(t_step) and call it in the same slot.
Risk: low–moderate, and verifiable — bubbles_euler is well covered by golden tests whose output lands in the bit-compared D/*.dat. A lift that preserves the write order (keep the helper call in the same position relative to the alpha block above and the Lagrange block below) is golden-safe; rebuild + run the bubble tests to confirm.
⛔ Out of scope: the FFT energy-spectrum block (lines 339–443)
The if (fft_wrt) block (the s_mpi_FFT_fwd calls, En/En_real accumulation, En_FFT_DATA/En_tot*.dat write) is also self-contained and looks liftable, but it has zero test coverage — no test or example sets fft_wrt=T, and its output file is not golden-compared. Refactoring untested output code is the worst risk/reward here. Do not lift it as part of this issue. If the FFT block is ever extracted, it should be paired with first adding an fft_wrt=T regression case to toolchain/mfc/test/cases.py.
Note / possible bundle with #1459
The FFT block's arrays (En, En_real, data_cmplx, data_cmplx_y, data_cmplx_z) are exactly the m_start_up.fpp arrays #1459 flags as @:ALLOCATEd-but-never-@:DEALLOCATEd. If/when the FFT block is touched (with a test in place), that's the natural moment to also fix the missing deallocations — but keep these arrays module-scoped (the block reads them as module state), don't localize them.
Optional stretch
The routine repeats a q_sf = ...; write(varname,...); call s_write_variable_to_formatted_database_file(varname, t_step); varname(:) = ' ' stanza ~30 times. A tiny s_write_field(varname, q_sf, t_step) helper would cut a lot of it — but that's a broader sweep across output code; treat as a follow-on, not the core ask.
Filed from a repo-wide code-cleanliness review; verified against master @ 40dde5e.
Code references
s_save_datainsrc/post_process/m_start_up.fppis a ~580-line routine (lines 169–747) that handles every post-processed output variable inline, plus two self-contained physics blocks that can be lifted into helpers to shrink the driver and isolate the machinery.In scope: lift the bubble-moment block
Lines 683–724 (the
if (bubbles_euler)moment block: thenR/nV/nP/nMloops overnbplus theadv_nnumber-density write) are a contiguous, self-contained unit sitting near the end of the routine.→ extract
s_write_bubble_moments(t_step)and call it in the same slot.Risk: low–moderate, and verifiable —
bubbles_euleris well covered by golden tests whose output lands in the bit-comparedD/*.dat. A lift that preserves the write order (keep the helper call in the same position relative to the alpha block above and the Lagrange block below) is golden-safe; rebuild + run the bubble tests to confirm.⛔ Out of scope: the FFT energy-spectrum block (lines 339–443)
The
if (fft_wrt)block (thes_mpi_FFT_fwdcalls,En/En_realaccumulation,En_FFT_DATA/En_tot*.datwrite) is also self-contained and looks liftable, but it has zero test coverage — no test or example setsfft_wrt=T, and its output file is not golden-compared. Refactoring untested output code is the worst risk/reward here. Do not lift it as part of this issue. If the FFT block is ever extracted, it should be paired with first adding anfft_wrt=Tregression case totoolchain/mfc/test/cases.py.Note / possible bundle with #1459
The FFT block's arrays (
En,En_real,data_cmplx,data_cmplx_y,data_cmplx_z) are exactly them_start_up.fpparrays #1459 flags as@:ALLOCATEd-but-never-@:DEALLOCATEd. If/when the FFT block is touched (with a test in place), that's the natural moment to also fix the missing deallocations — but keep these arrays module-scoped (the block reads them as module state), don't localize them.Optional stretch
The routine repeats a
q_sf = ...; write(varname,...); call s_write_variable_to_formatted_database_file(varname, t_step); varname(:) = ' 'stanza ~30 times. A tinys_write_field(varname, q_sf, t_step)helper would cut a lot of it — but that's a broader sweep across output code; treat as a follow-on, not the core ask.Filed from a repo-wide code-cleanliness review; verified against
master@40dde5e.Code references
src/post_process/m_start_up.fpp:169-747—s_save_data(~580 lines)src/post_process/m_start_up.fpp:683-724— bubble-moment block (in scope)src/post_process/m_start_up.fpp:339-443— FFT block (⛔ out of scope, untested)