Fix s_axis QBMM buffer reflection azimuthal index

[sbryngelson]

Fixes #1357.

Summary

• In s_axis(), the QBMM buffer reflection block always used l - ((p+1)/2) regardless of z_cc(l), while the primitive variable reflection correctly branches on z_cc(l) < pi
• For z_cc(l) < pi, this read from an incorrect (potentially negative) azimuthal index
• Added the same z_cc(l) < pi branch to the pb_in/mv_in QBMM block to mirror the primitive variable logic

Test plan

• 3D cylindrical QBMM simulation with axis BC (bc_y%beg = -14) and z_cc(l) < pi cells exercises the previously broken branch

Summary by CodeRabbit

• Bug Fixes
  • Improved axis boundary condition extrapolation for bubble pressure and vapor mass variables. The phase-shift direction is now conditionally selected based on geometric parameters for more accurate calculations.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0)   📘 Rule violations (1)   📎 Requirement gaps (0)   🎨 UX Issues (0)

📘\ ⚙ Maintainability (1)

1. Tab-indented if (z_cc) block 📘 ⚙

Description

The newly added if (z_cc(l) < pi) QBMM reflection block uses tab indentation rather than the
required 2-space indentation style. This reduces style consistency and violates the project’s
Fortran formatting conventions for modified code.

Code

src/common/m_boundary_common.fpp[R720-726]

+                        if (z_cc(l) < pi) then
+                            pb_in(k, -j, l, q, i) = pb_in(k, j - 1, l + ((p + 1)/2), q, i)
+                            mv_in(k, -j, l, q, i) = mv_in(k, j - 1, l + ((p + 1)/2), q, i)
+                        else
+                            pb_in(k, -j, l, q, i) = pb_in(k, j - 1, l - ((p + 1)/2), q, i)
+                            mv_in(k, -j, l, q, i) = mv_in(k, j - 1, l - ((p + 1)/2), q, i)
+                        end if

Evidence

PR Compliance ID 10 requires modified/new Fortran code to use 2-space indentation; the added lines
in the new QBMM branching block are indented with tabs (not 2-space indentation).

CLAUDE.md
src/common/m_boundary_common.fpp[720-726]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The added QBMM `if (z_cc(l) < pi)` block is tab-indented, but the compliance rule requires 2-space indentation for modified/new Fortran code.

## Issue Context
This is a style/maintainability compliance requirement for modified code.

## Fix Focus Areas
- src/common/m_boundary_common.fpp[720-726]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[github-actions]

Claude Code Review

Head SHA: 24f163b

Files changed:

• 1
• src/common/m_boundary_common.fpp

Findings

[Correctness] Hardcoded pi coordinate threshold is fragile and domain-dependent

In the modified hunk, the branch condition z_cc(l) < pi is used to decide which half of the z-domain a cell belongs to, choosing between l + (p+1)/2 and l - (p+1)/2 as the periodic mirror source index. This is almost certainly implementing a helical/shear-periodic boundary condition.

The problem is that z_cc(l) < pi only correctly bisects the domain when z runs from 0 to 2π. For any other domain extent (e.g. [0, 1], [-L, L], or any non-π-centered range), the threshold will fall in the wrong place, silently pairing ghost cells with the wrong interior source indices. This is a latent correctness bug for any case where the domain is not exactly [0, 2π].

A robust implementation should test the index directly rather than the physical coordinate:

if (l < (p + 1)/2) then
    pb_in(k, -j, l, q, i) = pb_in(k, j - 1, l + ((p + 1)/2), q, i)
    mv_in(k, -j, l, q, i) = mv_in(k, j - 1, l + ((p + 1)/2), q, i)
else
    pb_in(k, -j, l, q, i) = pb_in(k, j - 1, l - ((p + 1)/2), q, i)
    mv_in(k, -j, l, q, i) = mv_in(k, j - 1, l - ((p + 1)/2), q, i)
end if

This makes the split purely index-based (correct for any domain) and removes the dependency on the physical coordinate value and the pi constant.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

ⓘ The new review experience is currently in Beta. Learn more

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d41d5bbd-98c7-4556-9fa5-0960882788a6

📥 Commits

Reviewing files that changed from the base of the PR and between f6f5c85 and 24f163b.

📒 Files selected for processing (1)

• src/common/m_boundary_common.fpp

---

📝 Walkthrough

Walkthrough

The PR fixes a bug in the s_axis boundary condition subroutine where QBMM buffer extrapolation for bubble pressure and mass-vapor variables uses an incorrect azimuthal source index. The fix adds conditional branching based on z_cc(l) < pi to select between l + ((p + 1)/2) and l - ((p + 1)/2), mirroring the logic already implemented for primitive variable reflection.

Changes

Cohort / File(s)	Summary
QBMM Azimuthal Index Selection src/common/m_boundary_common.fpp	Added conditional branching for azimuthal phase-shift in QBMM pb_in and mv_in extrapolation based on z_cc(l) < pi, selecting appropriate shift direction to match primitive variable reflection logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Fix segfault in 3D cylindrical post-process boundary conditions #1356: Modifies QBMM extrapolation for pb_in and mv_in by making them optional with present() guards in the same subroutine.

Suggested labels

claude-full-review

Poem

🐰 The axis whispered a cylindrical tale,
Where phases shifted when pi's veil grows pale,
A bug in the QBMM's reflective dance,
Now branches wisely at each azimuthal glance!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically describes the main change: fixing the azimuthal index selection in the QBMM buffer reflection for s_axis when z_cc(l) < pi.
Description check	✅ Passed	The description includes the fixed issue number (#1357), a clear summary of the bug and fix, and a test plan. However, the test plan checklist items are unchecked.
Linked Issues check	✅ Passed	The PR directly addresses all requirements from issue #1357: adding z_cc(l) < pi branching to QBMM buffer reflection for pb_in and mv_in, preventing invalid azimuthal index reads.
Out of Scope Changes check	✅ Passed	All changes are scoped to the QBMM buffer reflection block in s_axis for pb_in and mv_in assignments; no unrelated modifications present.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.90%. Comparing base (23bbd4f) to head (24f163b).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1363      +/-   ##
==========================================
+ Coverage   64.88%   64.90%   +0.01%     
==========================================
  Files          70       70              
  Lines       18251    18254       +3     
  Branches     1508     1508              
==========================================
+ Hits        11842    11847       +5     
+ Misses       5446     5444       -2     
  Partials      963      963              

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