Fix 1D flow perturbation applying random factor twice (index aliasing)#1397
Fix 1D flow perturbation applying random factor twice (index aliasing)#1397sbryngelson wants to merge 1 commit intoMFlowCode:masterfrom
Conversation
…iasing In 1D, eqn_idx%mom%end == eqn_idx%mom%beg (same slot), so the old code wrote rand*v0 into the slot then read that modified value to compute (1+rand)*rand*v0, doubling the perturbation. Fix: save the original mom%beg value before any writes, scale mom%beg first, and only assign mom%end when num_vels > 1 (i.e., multi-D). Fixes MFlowCode#1369
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
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 |
Automated Code ReviewSummary: Fix is correct. One suggestion. StrengthsThe fix is mechanically correct:
SuggestionNo automated test covers |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1397 +/- ##
==========================================
- Coverage 64.75% 64.75% -0.01%
==========================================
Files 71 71
Lines 18721 18722 +1
Branches 1551 1552 +1
==========================================
Hits 12123 12123
- Misses 5640 5641 +1
Partials 958 958 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Fixes #1369.
In
s_perturb_surrounding_flow(src/pre_process/m_perturbation.fpp), the original code was:In 1D,
eqn_idx%mom%end == eqn_idx%mom%beg(same slot), so:rand * v0into the only velocity slot(1 + rand) * rand * v0The perturbation is applied twice, giving
rand*(1+rand)*v0instead of(1+rand)*v0.Fix
Save the original
mom%begvalue before any writes, scalemom%begfirst, then only assignmom%endwhennum_vels > 1:This also correctly preserves multi-D behaviour:
mom%endgetsrand * v_beg(the original x-velocity), notrand*(1+rand)*v_beg.Verification
Ran a 1D case (
m=9, v0=1.0, perturb_flow_mag=0.1) and read the post-pre_process velocity field. All 10 cell values fell in[1.0, 1.1]— consistent with(1 + rand) * v0whererand ∈ [0, 0.1]. Under the old bug the values would have been near zero (rand*(1+rand)*v0 ≪ 1). Not in the test suite, so verified manually.