Fix bc_x%ve3 never MPI-broadcast (duplicate ve2 instead)

[sbryngelson]

Summary

Severity: CRITICAL — non-root ranks get uninitialized bc_x%ve3.

File: src/simulation/m_mpi_proxy.fpp, line 148

The MPI broadcast list for x-boundary velocity-end parameters has bc_x%ve2 duplicated where bc_x%ve3 should be. The bc_y and bc_z rows are correct.

Before

& 'bc_x%vb1','bc_x%vb2','bc_x%vb3','bc_x%ve1','bc_x%ve2','bc_x%ve2', &
!                                                          ^^^^^^^^^ should be ve3
& 'bc_y%vb1','bc_y%vb2','bc_y%vb3','bc_y%ve1','bc_y%ve2','bc_y%ve3', &
& 'bc_z%vb1','bc_z%vb2','bc_z%vb3','bc_z%ve1','bc_z%ve2','bc_z%ve3', &

After

& 'bc_x%vb1','bc_x%vb2','bc_x%vb3','bc_x%ve1','bc_x%ve2','bc_x%ve3', &

Why this went undetected

Only manifests in multi-rank 3D runs with x-direction velocity boundary conditions that use the ve3 component — a narrow intersection of conditions.

Test plan

• Run multi-rank 3D simulation with x-boundary velocity BCs that set ve3
• Verify bc_x%ve3 is identical on root and non-root ranks

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Fixed an error in the MPI broadcast configuration where a boundary condition variable was incorrectly duplicated. The correction ensures all necessary boundary variables are properly synchronized during parallel simulation execution.

Fixes #1196

[codeant-ai]

CodeAnt AI is reviewing your PR.

---

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

[coderabbitai]

📝 Walkthrough

Walkthrough

A variable in the MPI user-input broadcast list is corrected by replacing a duplicate reference to bc_x%ve2 with bc_x%ve3, ensuring all three boundary condition components are properly included in the broadcast configuration.

Changes

Cohort / File(s)	Summary
MPI Broadcast Configuration src/simulation/m_mpi_proxy.fpp	Corrected broadcast variable list by replacing duplicate bc_x%ve2 with bc_x%ve3, ensuring all three velocity boundary condition components are included.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested labels

Review effort 1/5

Poem

A rabbit hops through code so fine,
Found a duplicate on line,
Ve-two became ve-three at last,
Broadcasting now—the fix is fast! 🐰✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: replacing a duplicated bc_x%ve2 with bc_x%ve3 in the MPI broadcast list.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	PR description provides excellent context with clear before/after code, severity assessment, and explanation of why the bug went undetected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codeant-ai]

Nitpicks 🔍

🔒 No security issues identified

⚡ Recommended areas for review Broadcast ordering / symmetry The broadcast list is positional and order-sensitive. Make sure the exact same list and order of MPI_BCAST calls is executed on all ranks and in any other module that expects these values. Any mismatch in ordering or count will make subsequent MPI_BCAST calls read the wrong memory locations and produce silent corruption. Type/Member consistency The PR adds broadcasting of bc_x%ve3. Confirm that the derived type defining bc_x actually contains ve3 (with the same kind/precision expected by mpi_p) and that it is initialized on rank 0 before the broadcast. If the member doesn't exist or has a different type/precision, the broadcast will corrupt data on receivers. Maintainability / Duplication risk Long explicit lists of nearly-identical variables (e.g. vb1..3, ve1..3 for x/y/z) are error-prone (duplicates/typos — exactly what this PR fixed). Consider generating these lists programmatically (the file already uses an fpp templating system); this reduces risk of future copy-paste errors.

[codeant-ai]

CodeAnt AI finished reviewing your PR.

[Copilot]

Pull request overview

This PR fixes a critical bug in MPI broadcast configuration where bc_x%ve2 was duplicated instead of including bc_x%ve3, causing the third velocity component boundary condition to remain uninitialized on non-root ranks in multi-rank 3D simulations.

Changes:

• Corrected the MPI broadcast list to include bc_x%ve3 instead of the duplicated bc_x%ve2

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.05%. Comparing base (84c46e0) to head (3ed586d).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1175   +/-   ##
=======================================
  Coverage   44.05%   44.05%           
=======================================
  Files          70       70           
  Lines       20498    20498           
  Branches     1990     1990           
=======================================
  Hits         9030     9030           
  Misses      10329    10329           
  Partials     1139     1139           

☔ 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.

[sbryngelson]

Superseded by #1242 (batched HPC-sensitive fixes)