Feature/mhd edge native dissipative flux (reduce ghost number constraint). - #1319
Open
UCaromel wants to merge 2 commits into
Open
Feature/mhd edge native dissipative flux (reduce ghost number constraint).#1319UCaromel wants to merge 2 commits into
UCaromel wants to merge 2 commits into
Conversation
The resistive and hyper-resistive contributions to F_B and F_Etot were built from face-side riemann-averaged quantities: the Laplacian of the face-averaged jt, multiplied by the reconstructed bt. Taking a Laplacian of a face quantity needs jt at face +/-1, which is why the Godunov flux loop grew by one layer in the flux direction under hyper-resistivity, and that shell is what forced the MHD ghost width to roundUpToEven(R+2). Compute them on the edge instead: E_diss = eta*J - nu*lapl(J) is formed where J natively lives, multiplied there by the transverse B projected onto the same edge, and the product is projected to the face with six new single-axis edge->face stencils. The product has to be formed before the projection since projection does not commute with multiplication. The Laplacian now stays within the reconstruction reach, so the flux direction grow is gone. It also makes these terms the face projection of the very quantities the CT already uses to advance B, where before the two used different discretisations of lapl(J) times different B's. The spatial hyper-resistivity coefficient stays face-side, assembled from the native normal B plus the two edge-projected transverse components, so no face->face projections are needed and rhot keeps its face meaning. Consequences: - bt_x/y/z and GodunovState exist only for the old face-side product and are removed, along with the fvm_state plumbing through the solver. - UpwindConstrainedTransportState gains a HyperResistivity parameter: the spatial coefficient reads rhot, which was gated on Hall || Resistivity, so a Hall-less resistivity-less spatial-hyper build read an unregistered view. Not bitwise when eta or nu is non-zero; identical when both are zero. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvDjQHGycauAiPgCgH2xFK
|
Warning Review limit reachedNext included review available in 54 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe MHD flux path now uses one constrained-transport state. Resistive and hyper-resistive terms are formed at edge locations, projected to faces, and accumulated into fluxes. Ghost-width calculations and tests reflect the revised stencil requirements. ChangesMHD flux refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR changes internal MHD numerical flux and ghost-cell handling without an identified merge-blocking correctness, security, deployment, or availability issue. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Godunov
participant GridLayoutImplYee
participant MHDEquations
Godunov->>Godunov: Compute edge-local resistive and hyper-resistive products
Godunov->>GridLayoutImplYee: Project edge products to face locations
Godunov->>MHDEquations: Accumulate projected terms into magnetic and energy fluxes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description references issue ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
UCaromel
force-pushed
the
feature/mhd-edge-native-dissipative-flux
branch
from
August 31, 2026 14:54
686aefa to
0f4e186
Compare
…is gone nbrGhostsFromReconstruction was roundUpToEven(R + 2). One of those layers paid for ampere computing J on the ghost box shrinked by one; the other paid for the hyper-resistivity flux shell, which no longer exists. WENOZ and MP5 go from 6 ghosts to 4; Linear, WENO3 and Constant are unchanged because the rounding to even absorbs the difference. The remaining margin is zero by construction, so state it: the Godunov class now static_asserts ghost_width >= nghosts + 1, which is what keeps the transverse grow shell reading valid J. The ghost width is one compile-time constant for the whole build, so in coupled runs this also relaxes the hierarchy's minimum patch size, which MHD was setting on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvDjQHGycauAiPgCgH2xFK
UCaromel
force-pushed
the
feature/mhd-edge-native-dissipative-flux
branch
from
August 31, 2026 14:59
0f4e186 to
8a26245
Compare
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
1 participant
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.
Issue
#1283
What this implements
nbrGhostsFromReconstruction<R>()wasroundUpToEven(R + 2), so WENOZ/MP5 MHD builds reserve 6 field ghost layers. The extra layer was bought by one thing: with hyper-resistivity on, laplJ was computed from reconstructed values, which required +1 cell in each directions.The non-ideal (resistive and hyper-resistive) flux contributions now come from J at its native edge location, so the Laplacian never reaches past layer 2, the
+1shell is gone, and the constant becomesroundUpToEven(R + 1): WENOZ/MP5 6 → 4, Linear/WENO3/Constant unchanged (rounding absorbs it).Design
E_diss = eta*J - coef*laplacian(J)is formed on the edge, multiplied there by the edge-projected transverse B — projection does not commute with multiplication, so the product must be formed before projecting — and the product is projected to the face. The flux's non-ideal terms are then the face projection of the same edge quantities the CT already uses to advance B, where before the two used different discretisations of ∇²J times different B's.Tests
Harris bitwise identical in ideal. Differences in the order of O(1e-5) with spatial hyper resistivity on harris at 0.4 dx. Expected since different numerics, lowering at O(dx^2) (hyper constant) or O(dx^4) (hyper spatial). Expected as second order representation of the laplacian (multiplied by dx^2 in spatial case).