Repository navigation
Fix the azimuthal flux divergence and geometric sources in 3D cylindrical coordinates - #2051
Open
aliencaocao wants to merge 3 commits into
Open
aliencaocao wants to merge 3 commits into
aliencaocao wants to merge 3 commits into
Conversation
The theta-direction flux difference is divided by r*dtheta alone; the extra cell-centre velocity factor is gone, and the four azimuthal volume-fraction source terms carry the same 1/(r*dtheta) metric. The HLL and Lax-Friedrichs theta sweeps write their own geometric momentum source, which the right-hand side turns into rho*u_theta^2/r for the radial momentum and -rho*u_r*u_theta/r for the azimuthal one, instead of leaving the radial sweep's source in the buffer. Adds three Azimuthal suite cases (HLL, HLLC, LF) with swirl and a theta-dependent wedge. The existing 3D cylindrical cases vary along the pipe only, so their goldens do not move. Fixes MFlowCode#1817.
|
Claude Code Review Head SHA: c8ecef2 Files changed:
Findings:
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2051 +/- ##
==========================================
+ Coverage 61.90% 61.96% +0.06%
==========================================
Files 86 86
Lines 22885 22895 +10
Branches 3369 3371 +2
==========================================
+ Hits 14166 14188 +22
+ Misses 6220 6205 -15
- Partials 2499 2502 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Lines of Code
|
wilfonba
approved these changes
Oct 15, 2026
wilfonba
left a comment
Collaborator
There was a problem hiding this comment.
This all appears to be correct. It also seems to fix some of the issues I was seeing with cylindrical coordinates several months ago. Merge away once the CI has run.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1817.
Changes
All changes are in the θ (z) direction of 3D cylindrical runs (
grid_geometry == 3):src/simulation/m_rhs.fpp: the flux divergence is(F(k−½) − F(k+½)) / (r Δθ). The extra factor of the cell-centre velocity u_θ is removed, and thevelocity_valprivate is removed with it.src/simulation/m_rhs.fpp: the four azimuthal volume fraction source terms use 1/(rΔθ) instead of 1/Δθ. The four terms are the per-fluid α trace and the shared velocity trace, each with its Kapila K∇·u part. Their metric matches the flux divergence next to them.src/simulation/m_riemann_solver_hll.fpp,m_riemann_solver_lf.fpp: the θ sweep writes its geometric momentum source instead of the value that the radial sweep leaves in the buffer. Radial momentum gets −(F_θ(ρu_θ) − p̂) = −ρu_θ², with p̂ the solver's own face pressure. Azimuthal momentum gets F_θ(ρu_r) = ρu_r u_θ. All other entries are zero. HLLC writes the same source in the same sweep. HLL and Lax-Friedrichs include pressure in their momentum flux. The new source removes that pressure with the same expression both solvers use in the radial sweep.The right-hand side applies −(1/r)·½(G(k−½) + G(k+½)) for this source G. The momentum equations therefore receive +ρu_θ²/r (radial) and −ρu_r u_θ/r (azimuthal).
The six-equation branches (θ phasic energy source, HLLC six-equation θ block) have the same omissions. The case validator rejects
model_eqns = 3in 3D cylindrical coordinates. Those branches are left unchanged.Why this is correct
The conservative form in cylindrical coordinates (vector and tensor divergence in Wikipedia: Del in cylindrical coordinates) is
F_θ already contains u_θ. The azimuthal difference is divided by rΔθ and multiplied by nothing. The non-conservative α∇·u term needs the same 1/(rΔθ). The radial sweep already cancels the p/r part of S(ρu_r). The θ sweep must therefore supply ρu_θ²/r and −ρu_r u_θ/r. HLL and Lax-Friedrichs include the pressure in their momentum flux. Their face pressure (s_M p_R − s_P p_L)/(s_M − s_P) is therefore subtracted. Both solvers use the same expression for the radial sweep.
Before and after
The first two figures show the uniform 40/60 mixture in a swirling pipe flow (HLLC) from the issue. Both cover one cross section of the pipe from t = 0 to 0.1:
The figure below shows solid body rotation with HLL, an exact steady state, at t = 1:
Tests and goldens
The existing 3D cylindrical cases vary only along the pipe. Their azimuthal differences vanish, and their goldens stay unchanged. Three cases are added (
3D -> Cylindrical -> Azimuthal -> HLL | HLLC | LF). Each case is the standard three-slab pipe with swirl u_θ = 0.1 and an off-axis wedge at a different pressure. The wedge is patch 4, which the Conduction case already uses. The cases share this patch through one dictionary. Pressure and swirl therefore vary with θ. All four patches hold one uniform 50/50 mixture. The cases therefore measure the azimuthal terms alone, whatever a solver does with volume fractions. The goldens were regenerated on CPU with GCC 16.2, in double precision, without case optimization:4C1D77D73D -> Cylindrical -> Azimuthal -> HLLC(new)6FEA09D23D -> Cylindrical -> Azimuthal -> HLL(new)FBD812663D -> Cylindrical -> Azimuthal -> LF(new)#1910 raises the same metric question for the viscous azimuthal flux. That term is computed elsewhere and is not changed here.
Contribution Policy
We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.
All contributions are expected to demonstrate:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.
Acknowledgement
PR template credit: junegunn