Skip to content

File-Per-Process IB markers - #1802

Draft
danieljvickers wants to merge 3 commits into
MFlowCode:masterfrom
danieljvickers:file-per-process-ib-markers
Draft

File-Per-Process IB markers#1802
danieljvickers wants to merge 3 commits into
MFlowCode:masterfrom
danieljvickers:file-per-process-ib-markers

Conversation

@danieljvickers

Copy link
Copy Markdown
Member

For some reason, I never added file-per-process for IB markers, and it is a slowdown constaint for some of the IB cases we are running. Adding this here. I tested this on the 2D_mibm_shock_cylinder case on 2 ranks and verified the visual output was correct.

Even though this is marked as "draft", it is ready for merge.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Claude Code Review

Head SHA: f0f2b3c

Files changed:

  • 2
  • src/post_process/m_data_input.f90
  • src/simulation/m_data_output.fpp

Findings:

  • src/post_process/m_data_input.f90:119-127: t_step_string is declared only inside the #ifdef MFC_MPI block (lines 114-120), but the new parallel_io .and. file_per_process branch that references it (call s_int_to_str(t_step, t_step_string) and the subsequent write/concatenation) sits outside any #ifdef MFC_MPI guard. In a non-MPI build this branch still has to compile (Fortran doesn't skip unreached branches), so t_step_string is an undeclared identifier there and the file fails to compile without MFC_MPI defined, breaking the CPU/non-MPI build path.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

Lines of Code

File Lines Diff
src/simulation/m_data_output.fpp 1326 +16
src/post_process/m_data_input.f90 420 +12
Directory Lines Diff
simulation 27684 +16
post_process 3339 +12
total 45466 +28

@danieljvickers

Copy link
Copy Markdown
Member Author

@sbryngelson Do you still want us to keep PRs as draft when they are done? This and the restart changes would both be useful for me to get merged. So it would be helpful to have them bumped up in terms of priority for merge if possible

@sbryngelson

Copy link
Copy Markdown
Member

noted. i'll prioritize it and move it out of draft soon enough

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The post-process source fails to compile without MPI because t_step_string is conditionally declared but unconditionally referenced.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds file-per-process output and post-processing support for immersed-boundary marker data.

Changes:

  • Writes rank-local IB marker files alongside timestep data.
  • Reads rank-local marker files during post-processing.
File summaries
File Description
src/simulation/m_data_output.fpp Writes per-rank IB marker files.
src/post_process/m_data_input.f90 Reads per-rank IB marker files.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +107 to 119
character(len=*), intent(in) :: file_loc_base
integer, intent(in), optional :: t_step
character(LEN=path_len + 2*name_len) :: file_loc
logical :: file_exist
integer :: ifile, ierr, data_size

#ifdef MFC_MPI
integer, dimension(MPI_STATUS_SIZE) :: status
integer(KIND=MPI_OFFSET_KIND) :: disp
integer(KIND=MPI_OFFSET_KIND) :: m_MOK, n_MOK, p_MOK, MOK, WP_MOK, var_MOK
integer :: save_index
character(len=10) :: t_step_string
#endif
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants