Skip to content

Fix acoustic source and body force timing #1232

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

Open
sbryngelson wants to merge 9 commits into
base: master
Choose a base branch
from
+4,121 −4,157
Open

Fix acoustic source and body force timing #1232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2690270
Fix acoustic source and body force timing
sbryngelson Feb 21, 2026
678a2dc
Regenerate golden files for body force adaptive-dt tests
sbryngelson Feb 22, 2026
9cf58ed
Regenerate golden files for all tests affected by timestep timing fix
sbryngelson Feb 22, 2026
bade5f4
Remove unused t_step parameter from s_acoustic_src_calculations
sbryngelson Feb 22, 2026
559c7e7
Regenerate golden metadata with MPI ON on Phoenix
sbryngelson Feb 26, 2026
aba125c
Fix misleading docstrings, typos, and add timing comments
sbryngelson Feb 26, 2026
ed98cb0
Merge branch 'master' into fix/time-stepping-order
sbryngelson Mar 12, 2026
d9bab25
Merge branch 'master' into fix/time-stepping-order
sbryngelson Mar 13, 2026
a3b2bb4
Merge branch 'master' into fix/time-stepping-order
sbryngelson Mar 14, 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
21 changes: 7 additions & 14 deletions src/simulation/m_acoustic_src.fpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,23 +124,16 @@ contains
end subroutine s_initialize_acoustic_src

!> This subroutine updates the rhs by computing the mass, mom, energy sources
!! @param t_step Current time step
!! @param q_cons_vf Conservative variables
!! @param q_prim_vf Primitive variables
!! @param rhs_vf rhs variables
impure subroutine s_acoustic_src_calculations(q_cons_vf, q_prim_vf, t_step, rhs_vf)
impure subroutine s_acoustic_src_calculations(q_cons_vf, q_prim_vf, rhs_vf)

type(scalar_field), dimension(sys_size), intent(inout) :: q_cons_vf !<
!! This variable contains the WENO-reconstructed values of the cell-average
!! conservative variables, which are located in q_cons_vf, at cell-interior
!! Gaussian quadrature points (QP).

type(scalar_field), dimension(sys_size), intent(inout) :: q_prim_vf !<
!! The primitive variables at cell-interior Gaussian quadrature points. These
!! are calculated from the conservative variables and gradient magnitude (GM)
!! of the volume fractions, q_cons_qp and gm_alpha_qp, respectively.
type(scalar_field), dimension(sys_size), intent(inout) :: q_cons_vf !< Conservative variables
type(scalar_field), dimension(sys_size), intent(inout) :: q_prim_vf !< Primitive variables

type(scalar_field), dimension(sys_size), intent(inout) :: rhs_vf

integer, intent(in) :: t_step
#:if not MFC_CASE_OPTIMIZATION and USING_AMD
real(wp), dimension(3) :: myalpha, myalpha_rho
#:else
Expand All @@ -165,7 +158,7 @@ contains

integer, parameter :: mass_label = 1, mom_label = 2

sim_time = t_step*dt
sim_time = mytime ! Accumulated time, correct under adaptive dt

$:GPU_PARALLEL_LOOP(private='[j,k,l]', collapse=3)
do l = 0, p
Expand All @@ -181,7 +174,7 @@ contains
end do
$:END_GPU_PARALLEL_LOOP()

! Keep outer loop sequel because different sources can have very different number of points
! Keep outer loop sequential because different sources can have very different number of points
do ai = 1, num_source
! Skip if the pulse has not started yet for sine and square waves
if (.not. (sim_time < delay(ai) .and. (pulse(ai) == 1 .or. pulse(ai) == 3))) then
Expand Down
2 changes: 1 addition & 1 deletion src/simulation/m_body_forces.fpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ contains

!> This subroutine calculates the mixture density at each cell
!! center
!! param q_cons_vf Conservative variable
!! @param q_cons_vf Conservative variables
subroutine s_compute_mixture_density(q_cons_vf)

type(scalar_field), dimension(sys_size), intent(in) :: q_cons_vf
Expand Down
5 changes: 2 additions & 3 deletions src/simulation/m_rhs.fpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1010,13 +1010,12 @@ contains
$:END_GPU_PARALLEL_LOOP()
end if

! Additional Physics and Source Temrs
! Additional Physics and Source Terms
! Additions for acoustic_source
if (acoustic_source) then
call nvtxStartRange("RHS-ACOUSTIC-SRC")
call s_acoustic_src_calculations(q_cons_qp%vf(1:sys_size), &
q_prim_qp%vf(1:sys_size), &
t_step, &
rhs_vf)
call nvtxEndRange
end if
Expand Down Expand Up @@ -1058,7 +1057,7 @@ contains

if (cont_damage) call s_compute_damage_state(q_cons_qp%vf, rhs_vf)

! END: Additional pphysics and source terms
! END: Additional physics and source terms

if (run_time_info .or. probe_wrt .or. ib .or. bubbles_lagrange) then
if (.not. igr .or. dummy) then
Expand Down
7 changes: 4 additions & 3 deletions src/simulation/m_start_up.fpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module m_start_up

use m_acoustic_src !< Acoustic source calculations

use m_rhs !< Right-hane-side (RHS) evaluation procedures
use m_rhs !< Right-hand-side (RHS) evaluation procedures

use m_chemistry !< Chemistry module

Expand Down Expand Up @@ -845,13 +845,14 @@ contains
end do
end if

mytime = mytime + dt

! Total-variation-diminishing (TVD) Runge-Kutta (RK) time-steppers
if (any(time_stepper == (/1, 2, 3/))) then
call s_tvd_rk(t_step, time_avg, time_stepper)
end if

! Advance time after RK so source terms see current-step time
mytime = mytime + dt

Comment on lines +853 to +855
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Pass a stage time into RHS/source evaluation instead of reusing global mytime.

Line 854 fixes the one-step offset, but mytime is still updated only once after the full RK step. src/simulation/m_time_steppers.fpp calls s_compute_rhs once per RK stage, while src/simulation/m_acoustic_src.fpp and src/simulation/m_body_forces.fpp both derive forcing time from mytime only. For SSP/TVD RK2/3, that keeps time-dependent forcing at t_n on every stage instead of t_n + c_s*dt, so acoustic/body-force integration remains first-order in time. Please thread an explicit stage time through RHS/source evaluation. As per coding guidelines, "PR-level guidelines emphasize timing correctness in RK sub-steps and the need to regenerate golden outputs for acoustic sources/body forces when timing semantics change."

if (relax) call s_infinite_relaxation_k(q_cons_ts(1)%vf)

! Time-stepping loop controls
Expand Down
133 changes: 69 additions & 64 deletions tests/016C1B8B/golden-metadata.txt
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions tests/016C1B8B/golden.txt
View file
Open in desktop

Large diffs are not rendered by default.

Loading
Loading