Repository navigation
Damp hyper-cleaning psi with the current-stage value - #2120
Draft
sbryngelson wants to merge 1 commit into
Draft
sbryngelson wants to merge 1 commit into
sbryngelson wants to merge 1 commit into
Conversation
The psi damping term in s_compute_rhs read q_prim_vf(psi), the time stepper's primitive array. That array is only refreshed at the end of s_compute_rhs, and only when run_time_info, probe_wrt, ib or bubbles_lagrange is set, so the damping used the previous stage's psi or, with none of those flags, a never-written array. Toggling run_time_info changed the solution. Use q_prim_qp%vf(psi), which aliases the current-stage conservative psi. run_time_info = T and F now give bit-identical output on a shrunk 2D_orszag_tang_hyper_cleaning case. Regenerated golden for B4DC99F9 (2D -> MHD -> hyper_cleaning); max abs deviation 3.9e-4.
sbryngelson
requested review from
danieljvickers
and removed request for
a team
November 3, 2026 03:26
sbryngelson
marked this pull request as draft
November 3, 2026 03:39
|
Claude Code Review Head SHA: 3260207 Files changed:
Findings: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2120 +/- ##
=======================================
Coverage 61.90% 61.90%
=======================================
Files 86 86
Lines 22948 22948
Branches 3386 3386
=======================================
Hits 14205 14205
Misses 6229 6229
Partials 2514 2514 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
What
The psi damping term in
s_compute_rhsreadq_prim_vf(psi), the time stepper's primitive array. That array is only refreshed at the end ofs_compute_rhs, and only whenrun_time_info,probe_wrt,iborbubbles_lagrangeis set, so the damping used the previous stage's psi or, with none of those flags, a never-written array. Togglingrun_time_infochanged the solution.Use
q_prim_qp%vf(psi), which aliases the current-stage conservative psi.Results change
Yes, for hyper-cleaning MHD cases.
run_time_info = TandFnow give bit-identical output on a shrunk2D_orszag_tang_hyper_cleaningcase (they differed before). The golden for B4DC99F9 (2D -> MHD -> hyper_cleaning) is regenerated; max abs deviation from the old golden is 3.9e-4.golden-metadata.txtis left unchanged.Verification
Testing. Built with GPU offload (OpenMP, AMD) and ran the full test suite with this fix and three other independent fixes applied together: 729 tests in 4 shards, 0 failures (three shards on MI300A, one on MI250X).
Prepared with Claude Code (AI-assisted); I reviewed the change and ran the tests.