Skip to content

Refactor /common and beta buffer routines for future particle inclusion #1672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
joshgillis wants to merge 14 commits into
base: master
Choose a base branch
from
+149 −171
Draft

Refactor /common and beta buffer routines for future particle inclusion #1672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
109ef34
Modifying beta storage in common to account for particle arrays.
joshgillis Jul 23, 2026
106324a
Merge branch 'MFlowCode:master' into add-particles
joshgillis Jul 23, 2026
1e166c8
Adding integers for halo expansion
joshgillis Jul 23, 2026
371d885
Merge branch 'refactor/particle-arrays' of https://github.com/UF-MFC/…
joshgillis Jul 23, 2026
5b5fe33
typos and minor changes to the MPI common module
joshgillis Jul 23, 2026
1787700
removing particles call until implementation of particles_lagrange
joshgillis Jul 23, 2026
eb5f415
removing particles lagrange here as well
joshgillis Jul 23, 2026
9be23da
adding beta_vars to bubble boundary routine calls
joshgillis Jul 23, 2026
19411ad
Merge branch 'MFlowCode:master' into refactor/particle-arrays
joshgillis Jul 23, 2026
ca611ea
Merge branch 'MFlowCode:master' into refactor/particle-arrays
joshgillis Jul 24, 2026
df0a969
formatting
Jul 24, 2026
8d48346
Merge branch 'master' into refactor/particle-arrays
joshgillis Jul 27, 2026
9fba977
remove s_beta_extrapolation (dead code)
joshgillis Jul 27, 2026
96e631b
Merge branch 'MFlowCode:master' into refactor/particle-arrays
joshgillis Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions src/common/m_boundary_common.fpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,34 +491,35 @@ contains
end subroutine s_finalize_boundary_common_module

!> Populate ghost cell buffers of the Lagrangian-bubble beta (void fraction) variables based on the boundary conditions.
impure subroutine s_populate_beta_buffers(q_beta, kahan_comp, bc_type, nvar)
impure subroutine s_populate_beta_buffers(q_beta, kahan_comp, bc_type, nvar, vars_comm)

type(scalar_field), dimension(:), intent(inout) :: q_beta
type(scalar_field), dimension(:), intent(inout) :: kahan_comp
type(integer_field), dimension(1:num_dims,1:2), intent(in) :: bc_type
integer, intent(in) :: nvar
integer, dimension(:), intent(in) :: vars_comm

call s_populate_beta_bc_direction(1, -1, bc%x, bc_type(1, 1), q_beta, kahan_comp, nvar)
call s_populate_beta_bc_direction(1, 1, bc%x, bc_type(1, 2), q_beta, kahan_comp, nvar)
call s_populate_beta_bc_direction(1, -1, bc%x, bc_type(1, 1), q_beta, kahan_comp, nvar, vars_comm)
call s_populate_beta_bc_direction(1, 1, bc%x, bc_type(1, 2), q_beta, kahan_comp, nvar, vars_comm)

! n > 0 always for bubbles_lagrange
#:if not MFC_CASE_OPTIMIZATION or num_dims > 1
call s_populate_beta_bc_direction(2, -1, bc%y, bc_type(2, 1), q_beta, kahan_comp, nvar)
call s_populate_beta_bc_direction(2, 1, bc%y, bc_type(2, 2), q_beta, kahan_comp, nvar)
call s_populate_beta_bc_direction(2, -1, bc%y, bc_type(2, 1), q_beta, kahan_comp, nvar, vars_comm)
call s_populate_beta_bc_direction(2, 1, bc%y, bc_type(2, 2), q_beta, kahan_comp, nvar, vars_comm)
#:endif

if (p == 0) return

#:if not MFC_CASE_OPTIMIZATION or num_dims > 2
call s_populate_beta_bc_direction(3, -1, bc%z, bc_type(3, 1), q_beta, kahan_comp, nvar)
call s_populate_beta_bc_direction(3, 1, bc%z, bc_type(3, 2), q_beta, kahan_comp, nvar)
call s_populate_beta_bc_direction(3, -1, bc%z, bc_type(3, 1), q_beta, kahan_comp, nvar, vars_comm)
call s_populate_beta_bc_direction(3, 1, bc%z, bc_type(3, 2), q_beta, kahan_comp, nvar, vars_comm)
#:endif

end subroutine s_populate_beta_buffers

!> Populate beta variable buffers for one direction and location, by dispatching the per-cell beta BC routines over the boundary
!! face and performing the paired MPI reduction for processor boundaries.
impure subroutine s_populate_beta_bc_direction(bc_dir, bc_loc, bc_bounds, bc_type_edge, q_beta, kahan_comp, nvar)
impure subroutine s_populate_beta_bc_direction(bc_dir, bc_loc, bc_bounds, bc_type_edge, q_beta, kahan_comp, nvar, vars_comm)

integer, intent(in) :: bc_dir, bc_loc
type(int_bounds_info), intent(in) :: bc_bounds
Expand All @@ -527,6 +528,7 @@ contains
type(scalar_field), dimension(:), intent(inout) :: kahan_comp
integer, intent(in) :: nvar
integer :: bc_edge, k_beg, k_end, l_beg, l_end, k, l, bc_code
integer, dimension(:), intent(in) :: vars_comm

if (bc_loc == -1) then
bc_edge = bc_bounds%beg
Expand All @@ -546,7 +548,7 @@ contains
l_beg = beta_bc_bounds(2)%beg; l_end = beta_bc_bounds(2)%end
end if

$:GPU_PARALLEL_LOOP(private='[l, k, bc_code]', collapse=2)
$:GPU_PARALLEL_LOOP(private='[l, k, bc_code]', collapse=2, copyin = '[vars_comm]')

@sbryngelson sbryngelson Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

beta_vars is already device-resident — $:GPU_DECLARE(create='[comm_coords, comm_size, beta_vars]') at m_mpi_common.fpp:34 and $:GPU_UPDATE(device='[beta_vars]') at m_mpi_common.fpp:102. Routing it through an assumed-shape dummy and adding copyin here replaces a device-resident read with a host-to-device transfer on every launch of this loop: six launches per timestep (two locations times three directions) in a hot BC path.

The payload is three integers, so this is not about bandwidth — it is the per-launch transfer and the descriptor handling for an assumed-shape dummy. Did the Tuolumne runs show any cost here?

A device-resident module-level array, or passing an integer selector instead of the array, would avoid it entirely.

Minor style note while here: copyin = '[vars_comm]' has spaces around = where every other clause in the file is written copyin='[...]'.

@joshgillis joshgillis Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm considering introducing a device-resident communication mapping (comm_vars) in m_mpi_common. The bubble routines would register beta_vars with this mapping, while the particle routines would register vars_send. The communication routines would then operate on comm_vars, avoiding the per-kernel copyin while also allowing the halo sizing to be driven by the active communication mapping. Do you think that this would be an improvement?

@sbryngelson sbryngelson Aug 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the copyin that is missing from the six equivalent kernels in s_mpi_reduce_beta_variables_buffers — see the inline note on m_mpi_common.fpp. Worth reconciling the two.

Minor, if you keep this approach: copyin on a per-launch basis in a routine that runs every timestep per direction adds a host-to-device transfer of a 3-element array each time. Negligible in absolute terms, but avoidable — a device-resident module table (option A) costs nothing per launch.

Also copyin = '[vars_comm]' has spaces around = where the other keyword arguments here don't.

do l = l_beg, l_end
do k = k_beg, k_end
! bc_type is not allocated over the beta ghost extents in x and y, so those directions dispatch on the
Expand All @@ -559,9 +561,9 @@ contains

select case (bc_code)
case (BC_PERIODIC)
call s_beta_periodic(q_beta, kahan_comp, bc_dir, bc_loc, k, l, nvar)
call s_beta_periodic(q_beta, kahan_comp, bc_dir, bc_loc, k, l, nvar, vars_comm)
case (BC_REFLECTIVE)
call s_beta_reflective(q_beta, kahan_comp, bc_dir, bc_loc, k, l, nvar)
call s_beta_reflective(q_beta, kahan_comp, bc_dir, bc_loc, k, l, nvar, vars_comm)
end select
end do
end do
Expand All @@ -571,7 +573,7 @@ contains
! The beta reduction is a paired exchange (rightward accumulate at bc_loc = -1, leftward distribute at bc_loc = 1), so it
! must run at both locations whenever either edge of the direction is a processor boundary.
if (bc_bounds%beg >= 0 .or. bc_bounds%end >= 0) then
call s_mpi_reduce_beta_variables_buffers(q_beta, kahan_comp, bc_dir, bc_loc, nvar)
call s_mpi_reduce_beta_variables_buffers(q_beta, kahan_comp, bc_dir, bc_loc, nvar, vars_comm)
end if

end subroutine s_populate_beta_bc_direction
Expand Down
Loading
Loading