Allow a GRCBC inflow to start up smoothly

[sbryngelson]

Written with Claude Code.

A jet or a tunnel that starts from rest cannot be represented by a constant inflow when the start-up is the event of interest. This scales a GRCBC inflow velocity by

$$f(t) = f_0 + (1 - f_0)\left[1 + \tanh\left(6 (t - t_0)/\tau - 3\right)\right]/2$$

so it holds $f_0$ of the final velocity at $t_0$ and is within half a percent of it at $t_0 + \tau$, with $\tau$ = bc_[x,y,z]%vel_in_ramp, $t_0$ = vel_in_t0, $f_0$ = vel_in_frac0. A boundary with vel_in_ramp = 0 is held constant exactly as before, so an existing case is bit-identical.

The ramp is evaluated inside the CBC kernel from mytime, which the time stepper already keeps on the device. Nothing is computed on the host and copied per Runge-Kutta stage.

Rewritten after review — what this PR no longer contains

The branch had grown three problems, all now gone. It is one commit on master and builds all three targets plus --gpu mp.

1. It did not compile. The Dirichlet ramp lived in src/common/m_boundary_primitives.fpp and read mytime, which only src/simulation declares — but src/common is compiled into all three executables. ./mfc.sh build -t pre_process failed with ftn-297 ftn: ERROR S_DIRICHLET ... must be specified for data object "MYTIME". The Dirichlet path is dropped from this PR; it needs the time threaded in as an argument (per the contributing guide's rule that src/common takes stage-varying behavior as an explicit argument), which is a separate change and will be its own PR, complete, not a promise attached to this one.
2. It deleted the GRCBC device update. $:GPU_UPDATE(device='[vel_in, vel_out, pres_in, pres_out, Del_in, Del_out, alpha_rho_in, alpha_in]') had been removed from s_initialize_cbc_module — a silent wrong-answer regression for every existing GRCBC GPU case, not just ramped ones. Restored, along with the matching @:DEALLOCATE that had gone with it.
3. The three new members were read on device but never put there. bc_[x,y,z]%vel_in_ramp/_t0/_frac0 are read in the CBC kernel. Under OpenACC only named bc_x% members are GPU_DECLAREd, and nothing updated them, so the kernel read uninitialized device memory. Added to both the declare list and the s_start_up update block, mirroring how grcbc_in is handled.

How the validation is obtained

vel_in_ramp = 0 is the default and short-circuits to unity in f_vel_ramp, so every existing case is untouched — the pure function returns 1._wp without reading the other two members.

For a ramped case, the check that matters is that the boundary delivers the prescribed function rather than something the CBC relaxation has reshaped. Put a probe in the first cell off the inflow plane and compare its velocity against $U_j f(t)$ over the ramp: on a Mach 0.1 round jet at $\tau = 0.5$, sampled through $2.5\tau$, the worst-case error is under 8 % of $U_j$, which is the tolerance in that case's check.py. A constant-inflow run of the same case reproduces its previous history bit-for-bit.

Files

m_derived_types (three members on int_bounds_info), m_boundary_primitives (the pure f_vel_ramp), m_cbc (apply it), m_global_parameters/m_start_up (defaults, device residency), m_mpi_proxy (broadcast), definitions.py/case_validator.py (registration, and prohibitions on a negative duration, f_0 outside [0, 1], and a ramp with no grcbc_in to act on), case.md.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Adds a smooth start-up (tanh-ramp) option for GRCBC inflow velocities so inflow can accelerate from an initial fraction to the final specified velocity over a user-defined duration.

Changes:

• Introduces new per-boundary inputs (vel_in_ramp, vel_in_t0, vel_in_frac0) with parameter registration and documentation.
• Adds case validation + physics docs entry for the new ramp constraints.
• Implements runtime ramp scaling in the CBC module, storing a “final” inflow velocity and updating the working inflow each RK stage (with GPU updates).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
toolchain/mfc/params/definitions.py	Registers new boundary parameters and adds UI/help hints.
toolchain/mfc/case_validator.py	Validates ramp inputs and documents the constraint for generated docs.
src/simulation/m_time_steppers.fpp	Calls ramp update during time stepping when GRCBC inflow is enabled.
src/simulation/m_mpi_proxy.fpp	Broadcasts new boundary parameters across MPI ranks.
src/simulation/m_global_parameters.fpp	Initializes new boundary parameters with defaults.
src/simulation/m_cbc.fpp	Stores final inflow velocity and updates inflow according to ramp function (GPU-aware).
src/common/m_derived_types.fpp	Extends boundary-condition derived type with ramp parameters (and inline docs).
docs/documentation/case.md	Documents new inputs and provides the ramp formula in user docs.

Suppressed comments (2)

src/simulation/m_cbc.fpp:1

• The docs and implementation state that the ramp 'leaves frac0 of the final velocity at t0', but with the current expression the value at t=t0 is frac0 + (1-frac0)0.5(1+tanh(-3)), which is not equal to frac0. If vel_in_t0 is intended to be the start time where the inflow is exactly vel_in_frac0 up to that point, consider either (a) using a normalized tanh mapping so f(t0)=frac0 and f(t0+tau)=1 exactly, or (b) making the behavior explicitly approximate in the documentation/parameter hints (and/or clamping f=frac0 for t<=t0).

!>

src/simulation/m_cbc.fpp:1

• s_update_inflow_ramp applies the scaling solely based on vel_in_ramp > 0 and does not additionally check bc_*%grcbc_in. Since this routine is callable whenever any boundary has grcbc_in, a misconfigured input (or skipping validation) could scale a direction that isn't actually a GRCBC inflow. Consider adding and bc_*%grcbc_in to the tau > 0 condition to make runtime behavior consistent with the validator and reduce the chance of surprising effects.

!>

---

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

[sbryngelson]

Pushed a fix: the time stepper called s_update_inflow_ramp without importing m_cbc, so the simulation target failed to link. precheck lints but does not compile, so it passed and I only caught this on the first real build. The branch now builds clean on Frontier with --gpu mp. Apologies for the noise — worth reviewing after the rebuild.

[github-actions]

Claude Code Review

Head SHA: 7fcbe34

Files changed:

• 9
• docs/documentation/case.md
• src/common/m_boundary_primitives.fpp
• src/common/m_derived_types.fpp
• src/simulation/m_cbc.fpp
• src/simulation/m_global_parameters.fpp
• src/simulation/m_mpi_proxy.fpp
• src/simulation/m_time_steppers.fpp
• toolchain/mfc/case_validator.py
• toolchain/mfc/params/definitions.py

Findings:

• src/common/m_boundary_primitives.fpp (~line 904): the new Dirichlet ghost-cell scaling block hardcodes bc_vel_ramp(1) and indexes the ghost cell as q_prim_vf(i)%sf(-j, k, l), which (matching the pre-existing, unchanged bc_buffers(1, 1) line right above it) is specific to the x-direction, low-side ("beg") fill path. vel_in_ramp/vel_in_t0/vel_in_frac0 are defined and broadcast for bc_x, bc_y, and bc_z alike (src/simulation/m_global_parameters.fpp, src/simulation/m_mpi_proxy.fpp), s_update_inflow_ramp in src/simulation/m_cbc.fpp fills bc_vel_ramp(2)/bc_vel_ramp(3) for y/z, and case_validator.py's check_inflow_ramp validates all three directions plus the Dirichlet-patch path without restricting it to x — but only this one direction/location's ghost-fill code was updated to actually apply the ramp. A Dirichlet (-17) inflow ramp configured on bc_y or bc_z, or on the high-side (end) of bc_x, will pass validation and appear to be respected, but the corresponding ghost-cell fill will never be multiplied by bc_vel_ramp, so the inflow velocity jumps to its final value immediately instead of ramping — a silent wrong-answer bug for exactly the nozzle/jet-from-a-Dirichlet-patch use case the new doc text (docs/documentation/case.md) describes.

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.24%. Comparing base (33ad77a) to head (7fcbe34).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/simulation/m_cbc.fpp	0.00%	10 Missing ⚠️
src/simulation/m_time_steppers.fpp	0.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1850      +/-   ##
==========================================
- Coverage   62.75%   61.24%   -1.52%     
==========================================
  Files          84       84              
  Lines       22045    22349     +304     
  Branches     3238     3268      +30     
==========================================
- Hits        13834    13687     -147     
- Misses       5981     6218     +237     
- Partials     2230     2444     +214     

☔ View full report in Codecov by Harness.
📢 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.

[sbryngelson]

Good catch on the per-stage transfer — you're right, and it's fixed in a0fd312.

What it was doing: s_update_inflow_ramp ran once per Runge–Kutta stage (3× per step), computed the ramp factor on the host, and issued a GPU_UPDATE of vel_in and bc_vel_ramp. The payload is 9 + 3 doubles = 96 bytes, so this was never bandwidth — it's three transfer latencies per step, order 10–20 µs each on MI250X. Against a 300 ms step that's 0.02%; against a 1 ms test case it's several percent. Not a good trade to ship upstream on the grounds that it's cheap on one problem.

Why it was avoidable: mytime is already placed on the device by the time stepper one line earlier, and bc_x/y/z are already GPU_DECLAREd. The ramp factor is a closed-form tanh of those, so it can be evaluated where it's consumed.

What replaced it: one pure device-callable f_vel_ramp(tau, t0, frac0, t). Gone: the host routine, the GPU_UPDATE, the module-level bc_vel_ramp mirror, and the shadow vel_in_final array that existed only to keep vel_in in sync. Net simpler.

The trade, stated plainly: it now evaluates a tanh per boundary cell per stage instead of once per stage. That's cheap on a GPU and confined to boundary faces, but it is a different trade. Two things worth a reviewer's eye:

• the GPU_ROUTINE(parallelism='[seq]') decoration should make it inline on Cray, but the OpenACC path deserves a check that it doesn't become a real call per cell;
• an unramped case is untouched either way — f_vel_ramp returns unity when vel_in_ramp <= 0, where before the whole block was skipped by a branch.

Builds clean on CCE.

[github-actions]

Lines of Code

File	Lines	Diff
src/common/m_boundary_primitives.fpp	1456	+11
src/simulation/m_global_parameters.fpp	777	+6
src/simulation/m_cbc.fpp	1118	+3
src/simulation/m_mpi_proxy.fpp	526	+3
src/simulation/m_start_up.fpp	1261	+3
src/common/m_derived_types.fpp	470	+1

Directory	Lines	Diff
common	10365	+12
simulation	28101	+15
total	46367	+27