Summary
In s_write_parallel_data_files (src/simulation/m_data_output.fpp), the file_per_process + down_sample path appears to write full-resolution field data using a downsampled element count when bubbles_euler is enabled. The non-bubbles path in the same if handles downsampling correctly, so the two branches disagree.
This is pre-existing on master (not introduced by any open PR). Filing to investigate rather than asserting a confirmed data-corruption bug, since I have not been able to execute the affected combination.
Detail
Inside if (file_per_process):
data_size is set from the downsampled extents when down_sample is on:
if (down_sample) then
data_size = (m_ds + 3)*(n_ds + 3)*(p_ds + 3)
else
data_size = (m + 1)*(n + 1)*(p + 1)
end if
MPI_IO_DATA%var is bound unconditionally to the full-size conservative fields:
call s_initialize_mpi_data(q_cons_vf, qbmm_pb=pb_ts(1), qbmm_mv=mv_ts(1))
! binds MPI_IO_DATA%var(i)%sf => q_cons_vf(i)%sf(0:m, 0:n, 0:p)
- The write then diverges by branch:
if (bubbles_euler) then
do i = 1, sys_size
! no down_sample case: full-size source, downsampled count
call MPI_FILE_WRITE_ALL(ifile, MPI_IO_DATA%var(i)%sf, data_size*mpi_io_type, mpi_io_p, status, ierr)
end do
...
else
if (down_sample) then
do i = 1, sys_size
! correct: writes the downsampled buffer
call MPI_FILE_WRITE_ALL(ifile, q_cons_temp_ds(i)%sf, data_size*mpi_io_type, mpi_io_p, status, ierr)
end do
else
...
end if
end if
The else branch switches its source array to q_cons_temp_ds when downsampling. The bubbles_euler branch does not, so with down_sample = T it writes the first (m_ds+3)*(n_ds+3)*(p_ds+3) elements of the full-resolution array — a contiguous prefix of the fine grid rather than the coarsened field.
The same concern applies to the qbmm .and. .not. polytropic sub-loop immediately below, which also writes MPI_IO_DATA%var with the downsampled data_size.
Reachability
The combination does not appear to be exercised:
examples/3D_IGR_33jet sets file_per_process = T and down_sample = T, but is IGR (no bubbles_euler), so it takes the correct else branch.
examples/3D_IGR_jet_1fluid sets file_per_process = T, down_sample = F.
I did not find a case combining bubbles_euler = T with down_sample = T and file_per_process = T.
Questions
- Is
bubbles_euler + down_sample + file_per_process a supported combination? If not, a @:PROHIBIT in the checker would be clearer than the current silent divergence.
- If supported, should the
bubbles_euler branch select q_cons_temp_ds under down_sample, mirroring the else branch?
- The two
! TODO: check if sys_size is correct comments on the surrounding loops suggest prior uncertainty about this routine and may be related.
Notes
Found while reviewing #1679; the region is byte-identical to master, so it is independent of that PR.
Summary
In
s_write_parallel_data_files(src/simulation/m_data_output.fpp), thefile_per_process+down_samplepath appears to write full-resolution field data using a downsampled element count whenbubbles_euleris enabled. The non-bubbles path in the sameifhandles downsampling correctly, so the two branches disagree.This is pre-existing on
master(not introduced by any open PR). Filing to investigate rather than asserting a confirmed data-corruption bug, since I have not been able to execute the affected combination.Detail
Inside
if (file_per_process):data_sizeis set from the downsampled extents whendown_sampleis on:MPI_IO_DATA%varis bound unconditionally to the full-size conservative fields:The
elsebranch switches its source array toq_cons_temp_dswhen downsampling. Thebubbles_eulerbranch does not, so withdown_sample = Tit writes the first(m_ds+3)*(n_ds+3)*(p_ds+3)elements of the full-resolution array — a contiguous prefix of the fine grid rather than the coarsened field.The same concern applies to the
qbmm .and. .not. polytropicsub-loop immediately below, which also writesMPI_IO_DATA%varwith the downsampleddata_size.Reachability
The combination does not appear to be exercised:
examples/3D_IGR_33jetsetsfile_per_process = Tanddown_sample = T, but is IGR (nobubbles_euler), so it takes the correctelsebranch.examples/3D_IGR_jet_1fluidsetsfile_per_process = T,down_sample = F.I did not find a case combining
bubbles_euler = Twithdown_sample = Tandfile_per_process = T.Questions
bubbles_euler+down_sample+file_per_processa supported combination? If not, a@:PROHIBITin the checker would be clearer than the current silent divergence.bubbles_eulerbranch selectq_cons_temp_dsunderdown_sample, mirroring theelsebranch?! TODO: check if sys_size is correctcomments on the surrounding loops suggest prior uncertainty about this routine and may be related.Notes
Found while reviewing #1679; the region is byte-identical to
master, so it is independent of that PR.