Skip to content

Conversation

wilfonba
Copy link
Collaborator

@wilfonba wilfonba commented Oct 9, 2025

User description

Description

This PR makes several improvements to I/O for Euler-Lagrange bubbles, run_time.inf,
and the progress output to the terminal in post process.

  • Euler-Lagrange bubble output
    • Adds optional header to bubbles.dat
    • Adds logicals to only write relevant bubble data
    • Adds bubble data to silo files for visualization in Paraview
    • Improve parallel output of bubble data to avoid reduction to root process
    • Added missing host updates for I/O of Lag bubbles. The host wasn't being updated
      for most of the bubble variables and the void fraction wasn't being reinitialized
      correctly so results were different when restarting from a save.
  • Updates to run_time.inf
    • Improves formatting for readability
    • Prints a number for Rc Min instead of asterisks
  • Updates to progress output in post process
    • Prints time/step and average time to terminal like simulation

Type of change

  • Something else

Scope

  • This PR comprises a set of related changes with a common goal

If you cannot check the above box, please split your PR into multiple PRs that each have a common goal.

How Has This Been Tested?

Euler-Lagrange bubble output

2D

The following video was created after adding lag_db_wrt to the examples/2D_lagrange_bubblescreen/
case file and running it with 4 MPI ranks on my laptop. The video shows the lagrangian
bubble data as spherical glyphs in Paraview that are scaled by the bubble radii.
The bubbles are shown 10x their actual size for visibility.

test.mp4

3D

The following video was created after adding lag_db_wrt to the examples/3D_lagrange_bubblescreen/
(I also increase the amplitude of the pulse to increase the radial dynamics)
case file and running it with 4 A100 GPUs on my Wingtip. The video shows the lagrangian
bubble data as spherical glyphs in Paraview that are scaled by the bubble radii.
The bubbles are shown 10x their actual size for visibility. I also ran this case with
4 MI250x GCDs on Frontier and got the same results. I also tried restarting the simulation
from an intermediate save and got the same results as running the simulation from
start to finish without a restart, both with parallel and serial I/O.

test.mp4

run_time.inf Verification

I ran viscous and inviscid cases and inspecte the output in run_time.inf. Comparisons
to the existing versions are in the drop down menus below.

Viscous cases

Old

Time-steps        Time         ICFL Max      VCFL Max        Rc Min
         0  0.011062              0.000000       0.112838       0.000001      **********
         1  0.011062              0.011062       0.112838       0.000001      **********
         2  0.011062              0.022125       0.112838       0.000001      **********
         3  0.011062              0.033187       0.112838       0.000001      **********
         4  0.011062              0.044250       0.112838       0.000001      **********

New

 Time-step                     dt                   Time               ICFL Max               VCFL Max                       Rc Min
         0               0.011062               0.011062               0.112838               0.000001                 1.446084E+05
         1               0.011062               0.022125               0.112838               0.000001                 1.446084E+05
         2               0.011062               0.033187               0.112838               0.000001                 1.446084E+05
         3               0.011062               0.044250               0.112838               0.000001                 1.446084E+05
         4               0.011062               0.055313               0.112838               0.000001                 1.446084E+05
Inviscid cases

Old

            Time-steps                Time                ICFL Max
                    0                0.011062                0.000000              0.112838
                    1                0.011062                0.011062              0.112838
                    2                0.011062                0.022125              0.112838
                    3                0.011062                0.033187              0.112838
                    4                0.011062                0.044250              0.112838

New

             Time-step                     dt                   Time               ICFL Max
                     0               0.011062               0.011062               0.112838
                     1               0.011062               0.022125               0.112838
                     2               0.011062               0.033187               0.112838
                     3               0.011062               0.044250               0.112838
                     4               0.011062               0.055313               0.112838

Post Process Verification CL Output

I ran a post process case and inspected the terminal output. It's correct

Checklist

  • I have added comments for the new code
  • I added Doxygen docstrings to the new code
  • I have made corresponding changes to the documentation (docs/)
  • I have added example cases in examples/ that demonstrate my new feature performing as expected.
    They run to completion and demonstrate "interesting physics"
  • I ran ./mfc.sh format before committing my code
  • New and existing tests pass locally with my changes, including with GPU capability enabled (both NVIDIA hardware with NVHPC compilers and AMD hardware with CRAY compilers) and disabled
  • This PR does not introduce any repeated code (it follows the DRY principle)
  • I cannot think of a way to condense this code and reduce any introduced additional line count

If your code changes any code source files (anything in src/simulation)

To make sure the code is performing as expected on GPU devices, I have:

  • Checked that the code compiles using NVHPC compilers
  • Checked that the code compiles using CRAY compilers
  • Ran the code on either V100, A100, or H100 GPUs and ensured the new feature performed as expected (the GPU results match the CPU results)
  • Ran the code on MI200+ GPUs and ensure the new features performed as expected (the GPU results match the CPU results)

PR Type

Enhancement


Description

  • Enhanced Euler-Lagrange bubble I/O with Silo database support

  • Improved run_time.inf formatting and terminal progress output

  • Added selective bubble data output controls

  • Fixed host memory updates for bubble restart functionality


Diagram Walkthrough

flowchart LR
  A["Bubble Data"] --> B["Text Output (.dat)"]
  A --> C["Silo Database Output"]
  D["Runtime Info"] --> E["Improved Formatting"]
  F["Post Process"] --> G["Enhanced Terminal Output"]
  H["Restart Files"] --> I["Fixed Host Updates"]
Loading

File Walkthrough

Relevant files
Configuration changes
6 files
m_constants.fpp
Added lag_io_vars constant for MPI I/O                                     
+1/-0     
m_global_parameters.fpp
Added Lagrangian bubble output control parameters               
+38/-0   
m_start_up.f90
Added bubble output parameters to input reading                   
+11/-6   
case.py
Enabled Silo database output for bubble data                         
+1/-0     
case.py
Enabled Silo database output for bubble data                         
+1/-0     
case_dicts.py
Added bubble output parameter definitions                               
+18/-0   
Enhancement
6 files
m_data_output.fpp
Enhanced bubble output with Silo database support               
+483/-55
m_mpi_proxy.fpp
Added MPI broadcast for bubble output parameters                 
+10/-1   
p_main.fpp
Added wall time measurement and progress reporting             
+13/-0   
m_bubbles.fpp
Added default case for bubble dynamics model                         
+4/-1     
m_bubbles_EL.fpp
Major improvements to bubble I/O and restart functionality
+248/-170
m_data_output.fpp
Improved run_time.inf formatting and output structure       
+14/-21 
Bug fix
2 files
m_assign_variables.fpp
Fixed surface tension variable assignment bug                       
+1/-1     
m_start_up.fpp
Fixed host memory updates for bubble variables                     
+4/-2     
Formatting
1 files
m_body_forces.fpp
Simplified acceleration computation with template loops   
+4/-8     
Documentation
1 files
case.md
Updated documentation for bubble output parameters             
+21/-3   

@Copilot Copilot AI review requested due to automatic review settings October 9, 2025 17:32
@wilfonba wilfonba requested review from a team and sbryngelson as code owners October 9, 2025 17:32
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements comprehensive I/O improvements for Euler-Lagrange bubbles, enhances the run_time.inf output formatting, and adds progress timing information to the post-process terminal output.

  • Adds configurable Lagrangian bubble output with new logical parameters for selective data writing
  • Improves run_time.inf formatting with better column alignment and proper numeric output for Rc Min
  • Adds wall time measurements and progress indicators to post-process output similar to simulation

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
toolchain/mfc/run/case_dicts.py Adds new Lagrangian bubble output control parameters
src/simulation/m_start_up.fpp Fixes GPU host updates for bubble variables and removes redundant update
src/simulation/m_data_output.fpp Improves run_time.inf formatting with better column alignment
src/simulation/m_bubbles_EL.fpp Major improvements to bubble I/O, restart functionality, and boundary condition handling
src/simulation/m_bubbles.fpp Adds default case for bubble dynamics and fixes comment
src/simulation/m_body_forces.fpp Refactors body force computation using Fypp templates
src/pre_process/m_assign_variables.fpp Fixes surface tension variable assignment
src/post_process/p_main.fpp Adds wall time measurements for progress tracking
src/post_process/m_start_up.f90 Updates progress output format with timing information
src/post_process/m_mpi_proxy.fpp Adds MPI broadcasting for new Lagrangian bubble parameters
src/post_process/m_global_parameters.fpp Defines new Lagrangian bubble output control variables
src/post_process/m_data_output.fpp Splits bubble output into separate text and database functions with selective variable writing
src/common/m_constants.fpp Adds constant for Lagrangian I/O variable count
examples/*/case.py Enables Lagrangian database output in example cases
docs/documentation/case.md Documents new Lagrangian bubble output parameters

Copy link

qodo-merge-pro bot commented Oct 9, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

In text writer, the velocity output uses inputvals(7), inputvals(8), inputvals(8) for vx, vy, vz, duplicating vy and omitting vz. This likely should be 7, 8, 9. Verify all column indices match the MPI I/O layout consistently.

if (lag_pos_wrt) write (29, '(3(E15.7, A))', advance='no') inputvals(1), ', ', inputvals(2), ', ', inputvals(3), ', '
if (lag_pos_prev_wrt) write (29, '(3(E15.7, A))', advance='no') inputvals(4), ', ', inputvals(5), ', ', inputvals(6), ', '
if (lag_vel_wrt) write (29, '(3(E15.7, A))', advance='no') inputvals(7), ', ', inputvals(8), ', ', inputvals(8), ', '
if (lag_rad_wrt) write (29, '(E15.7, A)', advance='no') inputvals(10), ', '
Boundary Check Change

The previous clamping of ghost-cell indices to the nearest in-domain cell is now commented out and replaced by aborts for multiple wall-like BCs. Confirm this stricter behavior is intended for all cases and won’t break valid restart scenarios near boundaries.

!cell(1) = min(max(cell(1), 0), m)
!cell(2) = min(max(cell(2), 0), n)
!if (p > 0) cell(3) = min(max(cell(3), 0), p)
call s_convert_to_mixture_variables(q_cons_vf, cell(1), cell(2), cell(3), &
MPI I/O Header Consistency

New restart file header now includes tot_part, time, dt, num_procs, and per-rank counts. Ensure all readers/writers (simulation and post_process) handle offsets, types, and sizeof consistently across platforms (INTEGER sizes) and that non-participating ranks use a zero-length view without deadlocks.

! Construct file path
write (file_loc, '(A,I0,A)') 'lag_bubbles_', t_step, '.dat'
file_loc = trim(case_dir)//'/restart_data'//trim(mpiiofs)//trim(file_loc)

! Check if file exists
inquire (FILE=trim(file_loc), EXIST=file_exist)
if (.not. file_exist) then
    call s_mpi_abort('Restart file '//trim(file_loc)//' does not exist!')
end if

if (.not. parallel_io) return

if (proc_rank == 0) then
    call MPI_FILE_OPEN(MPI_COMM_SELF, file_loc, MPI_MODE_RDONLY, &
                       mpi_info_int, ifile, ierr)

    call MPI_FILE_READ(ifile, file_tot_part, 1, MPI_INTEGER, status, ierr)
    call MPI_FILE_READ(ifile, file_time, 1, mpi_p, status, ierr)
    call MPI_FILE_READ(ifile, file_dt, 1, mpi_p, status, ierr)
    call MPI_FILE_READ(ifile, file_num_procs, 1, MPI_INTEGER, status, ierr)

    call MPI_FILE_CLOSE(ifile, ierr)
end if

call MPI_BCAST(file_tot_part, 1, MPI_INTEGER, 0, MPI_COMM_WORLD, ierr)
call MPI_BCAST(file_time, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)
call MPI_BCAST(file_dt, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)
call MPI_BCAST(file_num_procs, 1, MPI_INTEGER, 0, MPI_COMM_WORLD, ierr)

allocate (proc_bubble_counts(file_num_procs))

if (proc_rank == 0) then
    call MPI_FILE_OPEN(MPI_COMM_SELF, file_loc, MPI_MODE_RDONLY, &
                       mpi_info_int, ifile, ierr)

    ! Skip to processor counts position
    disp = int(sizeof(file_tot_part) + 2*sizeof(file_time) + sizeof(file_num_procs), &
               MPI_OFFSET_KIND)
    call MPI_FILE_SEEK(ifile, disp, MPI_SEEK_SET, ierr)
    call MPI_FILE_READ(ifile, proc_bubble_counts, file_num_procs, MPI_INTEGER, status, ierr)

    call MPI_FILE_CLOSE(ifile, ierr)
end if

call MPI_BCAST(proc_bubble_counts, file_num_procs, MPI_INTEGER, 0, MPI_COMM_WORLD, ierr)

gsizes(1) = file_tot_part
gsizes(2) = lag_io_vars
lsizes(1) = file_tot_part
lsizes(2) = lag_io_vars
start_idx_part(1) = 0
start_idx_part(2) = 0

call MPI_TYPE_CREATE_SUBARRAY(2, gsizes, lsizes, start_idx_part, &
                              MPI_ORDER_FORTRAN, mpi_p, view, ierr)
call MPI_TYPE_COMMIT(view, ierr)

call MPI_FILE_OPEN(MPI_COMM_WORLD, file_loc, MPI_MODE_RDONLY, &
                   mpi_info_int, ifile, ierr)

disp = int(sizeof(file_tot_part) + 2*sizeof(file_time) + sizeof(file_num_procs) + &
           file_num_procs*sizeof(proc_bubble_counts(1)), MPI_OFFSET_KIND)
call MPI_FILE_SET_VIEW(ifile, disp, mpi_p, view, &
                       'native', mpi_info_null, ierr)

allocate (MPI_IO_DATA_lg_bubbles(file_tot_part, 1:lag_io_vars))

@wilfonba
Copy link
Collaborator Author

wilfonba commented Oct 9, 2025

I'm not sure why, but the MacOS runners are failing due to some Python version incompatibility with Cantera

Copy link

codecov bot commented Oct 9, 2025

Codecov Report

❌ Patch coverage is 15.90909% with 370 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.14%. Comparing base (c86fdd9) to head (eea226f).

Files with missing lines Patch % Lines
src/post_process/m_data_output.fpp 0.00% 256 Missing and 1 partial ⚠️
src/simulation/m_bubbles_EL.fpp 25.69% 105 Missing and 2 partials ⚠️
src/post_process/m_start_up.f90 50.00% 2 Missing ⚠️
src/post_process/m_mpi_proxy.fpp 50.00% 0 Missing and 1 partial ⚠️
src/pre_process/m_assign_variables.fpp 0.00% 1 Missing ⚠️
src/simulation/m_body_forces.fpp 50.00% 0 Missing and 1 partial ⚠️
src/simulation/m_data_output.fpp 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1015      +/-   ##
==========================================
- Coverage   41.75%   41.14%   -0.61%     
==========================================
  Files          70       70              
  Lines       20126    20387     +261     
  Branches     2504     2576      +72     
==========================================
- Hits         8403     8389      -14     
- Misses      10180    10470     +290     
+ Partials     1543     1528      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

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

Successfully merging this pull request may close these issues.

1 participant