Skip to content

Comments

Fix grid stretching using wrong array bounds for y_cc and z_cc#1179

Closed
sbryngelson wants to merge 1 commit intoMFlowCode:masterfrom
sbryngelson:fix/grid-stretching-bounds
Closed

Fix grid stretching using wrong array bounds for y_cc and z_cc#1179
sbryngelson wants to merge 1 commit intoMFlowCode:masterfrom
sbryngelson:fix/grid-stretching-bounds

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Feb 21, 2026

Summary

Severity: CRITICAL — corrupts y/z cell-center coordinates when grid stretching is active.

File: src/pre_process/m_grid.f90, lines 134 and 171

After applying grid stretching, the cell-center arrays are computed with the wrong left-hand side bounds: y_cc(0:m) and z_cc(0:m) use the x-dimension size m instead of the y-dimension n and z-dimension p.

Before

! y-direction (line 134)
y_cc(0:m) = (y_cb(0:n) + y_cb(-1:n-1))/2._wp   ! m is x-size, should be n

! z-direction (line 171)
z_cc(0:m) = (z_cb(0:p) + z_cb(-1:p-1))/2._wp   ! m is x-size, should be p

After

y_cc(0:n) = (y_cb(0:n) + y_cb(-1:n-1))/2._wp
z_cc(0:p) = (z_cb(0:p) + z_cb(-1:p-1))/2._wp

Why this went undetected

When m == n == p (equal resolution in all directions), the wrong index produces the same result. The bug only manifests with anisotropic grids and stretching enabled.

Test plan

  • Run 3D case with grid stretching and anisotropic resolution (m != n != p)
  • Verify y_cc and z_cc cell centers match expected stretched coordinates

🤖 Generated with Claude Code

Fixes #1200

Copilot AI review requested due to automatic review settings February 21, 2026 03:23
@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

Warning

Rate limit exceeded

@sbryngelson has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 28 minutes and 27 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

✨ 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
@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 21, 2026

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Array bounds correction
    The PR fixes y/z cell-center computation by using the correct dimension sizes for the slices used to compute cell centers. Verify that the arrays have the expected lower bounds (including the -1 boundary) and sizes so the slices (0:n) and (-1:n-1) / (0:p) and (-1:p-1) are valid for all build configurations and array declarations. Also validate behavior for degenerate sizes (n==0 or p==0) and MPI-parallel cases.

  • In-place scaling side-effect
    The code divides y_a, y_b, z_a, and z_b in-place by length. That alters the original parameter values for the rest of the routine/module. Confirm this side-effect is intended; if not, use temporaries to avoid mutating the original scaling parameters.

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 21, 2026

CodeAnt AI finished reviewing your PR.

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 incorrect array section bounds used when computing cell-center coordinates (y_cc, z_cc) during grid stretching, which corrupted coordinates for anisotropic grids.

Changes:

  • Use n (y dimension) instead of m when assigning y_cc(0:...).
  • Use p (z dimension) instead of m when assigning z_cc(0:...).

cubic-dev-ai[bot]

This comment was marked as off-topic.

y_cc(0:m) and z_cc(0:m) use the x-dimension size m instead of the
y-dimension n and z-dimension p respectively. Corrupts cell-center
coordinates when grid stretching is enabled and m != n or m != p.

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

codecov bot commented Feb 21, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.05%. Comparing base (84c46e0) to head (b95f40c).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/pre_process/m_grid.f90 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1179   +/-   ##
=======================================
  Coverage   44.05%   44.05%           
=======================================
  Files          70       70           
  Lines       20498    20498           
  Branches     1990     1990           
=======================================
  Hits         9030     9030           
  Misses      10329    10329           
  Partials     1139     1139           

☔ 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
Copy link
Member Author

Superseded by #1241 (batched low-risk fixes)

@sbryngelson sbryngelson deleted the fix/grid-stretching-bounds branch February 22, 2026 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files

Development

Successfully merging this pull request may close these issues.

MUSCL THINC right-state reconstruction uses already-overwritten left-state values

1 participant