Skip to content

Comments

Fix hyper_cleaning psi initialization only covering k=0 plane#1217

Open
sbryngelson wants to merge 3 commits intoMFlowCode:masterfrom
sbryngelson:fix/hyper-cleaning-3d-init
Open

Fix hyper_cleaning psi initialization only covering k=0 plane#1217
sbryngelson wants to merge 3 commits intoMFlowCode:masterfrom
sbryngelson:fix/hyper-cleaning-3d-init

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Feb 21, 2026

User description

Summary

  • Fix MHD hyperbolic divergence cleaning (hyper_cleaning) psi field initialization in m_start_up.fpp to cover all z-planes in 3D simulations, not just the z=0 plane.

Bug Details

The psi initialization loop hardcodes the third array index to 0:

do j = 0, m
    do k = 0, n
        q_cons_vf(psi_idx)%sf(j, k, 0) = ...   ! only z=0 plane
    end do
end do

In 3D simulations (p > 0), all z-planes beyond z=0 are left uninitialized (containing whatever value they had from prior initialization or garbage). This corrupts the divergence cleaning field in 3D MHD simulations, as the psi field would be zero (or uninitialized) everywhere except the first z-plane.

Fix

Add an outer loop over the z-dimension (l = 0, p) and include z_cc(l) in the Gaussian exponent so the 3D initialization is physically consistent. For 2D cases (p=0), the loop executes once with l=0, preserving existing behavior.

Test plan

  • 2D MHD cases with hyper_cleaning produce identical results (regression)
  • 3D MHD cases with hyper_cleaning have properly initialized psi field across all z-planes

🤖 Generated with Claude Code


CodeAnt-AI Description

Initialize hyperbolic cleaning psi across all z-planes in 3D

What Changed

  • The hyperbolic-divergence-cleaning psi field is now initialized for every z-plane in 3D runs instead of only the z=0 plane.
  • The Gaussian initialization now uses the z-coordinate so the psi field is physically consistent across depth.
  • 2D behavior is unchanged (single z-plane initialization preserved).

Impact

✅ Fewer uninitialized psi values in 3D initial conditions
✅ Fewer 3D divergence-cleaning artifacts during startup
✅ Preserves existing 2D initialization behavior

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Fixes #1221

Summary by CodeRabbit

  • Refactor
    • Extended startup initialization routine to support 3D grid configurations in addition to 2D grids.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 21, 2026 05:03
@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 21, 2026

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
Copy link
Contributor

coderabbitai bot commented Feb 21, 2026

📝 Walkthrough

Walkthrough

The hyper_cleaning initialization for the psi field is expanded from a 2D loop covering only the z=0 plane to a 3D loop iterating over all z-planes. The Gaussian expression now incorporates the z coordinate when p > 0, while maintaining backward compatibility for 2D simulations.

Changes

Cohort / File(s) Summary
Hyper-cleaning 3D initialization
src/pre_process/m_start_up.fpp
Extended psi field initialization from 2D (j, k, 0) to full 3D (j, k, l) with outer loop over z-dimension. Gaussian expression updated to include z_cc(l) term when p > 0, with fallback to 2D form when p == 0. Both conservative and primitive fields updated consistently.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A rabbit's ode to divergence tamed,
Where psi fields once suffered shame,
Now z-planes all get their due,
Gaussians bloom in dimension true!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary change: fixing psi initialization in hyper_cleaning to cover all z-planes instead of just k=0.
Description check ✅ Passed The description comprehensively covers the bug, fix, and test plan, with clear before/after code examples and detailed rationale for the changes.
Linked Issues check ✅ Passed The code changes directly address all objectives from issue #1221: adding z-loop over l=0..p, including z_cc(l) in Gaussian, and preserving 2D behavior when p=0.
Out of Scope Changes check ✅ Passed All changes are narrowly scoped to the psi initialization loop in m_start_up.fpp and directly address the hyper_cleaning bug without introducing unrelated modifications.
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
  • 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

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

@codeant-ai codeant-ai bot added the size:XS This PR changes 0-9 lines, ignoring generated files label Feb 21, 2026
cubic-dev-ai[bot]

This comment was marked as off-topic.

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 21, 2026

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Initialization Coverage
    The new triple loop initializes psi across z-planes (good), but reviewers should verify that p, m, n are the intended loop bounds in all execution paths (especially when reading existing ICs or when domain decomposition changes local vs global indexing). Confirm that psi_idx is valid and that no other code expects a different initialization order or value for any plane.

  • Precision Consistency
    The numeric literals in the new expression (e.g. 1d-2, 2.0, 0.05) may mix default-kind and kind-wp values. Ensure the constants match real(wp) precision so there is no unintended down-/up-casting or precision loss across platforms.

  • Performance / Redundant Work
    The Gaussian denominator 2.0*0.05**2 is recomputed for every cell and the exponent is evaluated repeatedly. For large grids this is unnecessary work; precomputing constant terms (and using array-slice copies) would reduce work and improve cache behavior.

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 21, 2026

CodeAnt AI finished reviewing your PR.

@sbryngelson sbryngelson added the bug Something isn't working or doesn't seem right label Feb 21, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes MHD hyperbolic divergence cleaning (hyper_cleaning) psi initialization so 3D simulations initialize all z-planes (not just z=0), making the Gaussian initialization spatially consistent in 3D.

Changes:

  • Add a z-loop (l = 0, p) for psi initialization.
  • Include z_cc(l) in the Gaussian exponent and initialize sf(j,k,l) for all planes.

sbryngelson and others added 2 commits February 21, 2026 00:11
z_cc is only allocated when p > 0. The previous commit accessed z_cc(l)
unconditionally, which would crash or read garbage in 2D runs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace `1d-2` (hard double precision) and bare `2.0`/`0.05` (default
real) with `_wp`-suffixed literals so the Gaussian initialization is
consistent with the working precision across single and double builds.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/pre_process/m_start_up.fpp (1)

789-801: Hoist the loop-invariant if (p > 0) branch outside all three loops.

p is a global constant — the branch in the innermost loop is re-evaluated (m+1)×(n+1)×(p+1) times unnecessarily, and the intent is clearer when the 3D vs. 2D paths are separated at the outermost level.

♻️ Proposed refactor
-            do l = 0, p
-                do k = 0, n
-                    do j = 0, m
-                        if (p > 0) then
-                            q_cons_vf(psi_idx)%sf(j, k, l) = 1.0e-2_wp*exp(-(x_cc(j)**2 + y_cc(k)**2 + z_cc(l)**2)/(2.0_wp*0.05_wp**2))
-                        else
-                            q_cons_vf(psi_idx)%sf(j, k, l) = 1.0e-2_wp*exp(-(x_cc(j)**2 + y_cc(k)**2)/(2.0_wp*0.05_wp**2))
-                        end if
-                        q_prim_vf(psi_idx)%sf(j, k, l) = q_cons_vf(psi_idx)%sf(j, k, l)
-                    end do
-                end do
-            end do
+            if (p > 0) then
+                do l = 0, p
+                    do k = 0, n
+                        do j = 0, m
+                            q_cons_vf(psi_idx)%sf(j, k, l) = 1.0e-2_wp*exp(-(x_cc(j)**2 + y_cc(k)**2 + z_cc(l)**2)/(2.0_wp*0.05_wp**2))
+                            q_prim_vf(psi_idx)%sf(j, k, l) = q_cons_vf(psi_idx)%sf(j, k, l)
+                        end do
+                    end do
+                end do
+            else
+                do k = 0, n
+                    do j = 0, m
+                        q_cons_vf(psi_idx)%sf(j, k, 0) = 1.0e-2_wp*exp(-(x_cc(j)**2 + y_cc(k)**2)/(2.0_wp*0.05_wp**2))
+                        q_prim_vf(psi_idx)%sf(j, k, 0) = q_cons_vf(psi_idx)%sf(j, k, 0)
+                    end do
+                end do
+            end if
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pre_process/m_start_up.fpp` around lines 789 - 801, The innermost branch
testing p (>0) should be hoisted outside the triple loop to avoid re-evaluating
a loop-invariant; when hyper_cleaning is true, split into two separate nested
loops: one for the 3D case that uses z_cc(l) and sets q_cons_vf(psi_idx)%sf and
q_prim_vf(psi_idx)%sf with exp(-(x_cc(j)**2 + y_cc(k)**2 + z_cc(l)**2)/(...)),
and one for the 2D case that omits z_cc(l) and uses exp(-(x_cc(j)**2 +
y_cc(k)**2)/(...)); keep the existing hyper_cleaning guard and the assignments
to q_cons_vf and q_prim_vf and reference psi_idx, x_cc, y_cc, z_cc, m, n, p to
locate and restructure the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/pre_process/m_start_up.fpp`:
- Around line 789-801: The innermost branch testing p (>0) should be hoisted
outside the triple loop to avoid re-evaluating a loop-invariant; when
hyper_cleaning is true, split into two separate nested loops: one for the 3D
case that uses z_cc(l) and sets q_cons_vf(psi_idx)%sf and q_prim_vf(psi_idx)%sf
with exp(-(x_cc(j)**2 + y_cc(k)**2 + z_cc(l)**2)/(...)), and one for the 2D case
that omits z_cc(l) and uses exp(-(x_cc(j)**2 + y_cc(k)**2)/(...)); keep the
existing hyper_cleaning guard and the assignments to q_cons_vf and q_prim_vf and
reference psi_idx, x_cc, y_cc, z_cc, m, n, p to locate and restructure the code.

@codecov
Copy link

codecov bot commented Feb 21, 2026

Codecov Report

❌ Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.05%. Comparing base (84c46e0) to head (116d256).

Files with missing lines Patch % Lines
src/pre_process/m_start_up.fpp 33.33% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1217      +/-   ##
==========================================
- Coverage   44.05%   44.05%   -0.01%     
==========================================
  Files          70       70              
  Lines       20498    20501       +3     
  Branches     1990     1991       +1     
==========================================
+ Hits         9030     9031       +1     
- Misses      10329    10330       +1     
- Partials     1139     1140       +1     

☔ 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 sbryngelson requested review from ChrisZYJ, anandrdbz, danieljvickers and wilfonba and removed request for anandrdbz February 21, 2026 14:11
@ChrisZYJ
Copy link
Contributor

@sbryngelson Thanks for fixing my oversight. The new 3D initialization looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working or doesn't seem right size:XS This PR changes 0-9 lines, ignoring generated files

Development

Successfully merging this pull request may close these issues.

hyper_cleaning psi initialization only covers z=0 plane in 3D

2 participants