Skip to content

fix: Enforce strict grid convergence monotonicity now that CG Poisson…#172

Merged
shaia merged 3 commits into
masterfrom
fix/grid-convergence-strict-monotonicity
Mar 6, 2026
Merged

fix: Enforce strict grid convergence monotonicity now that CG Poisson…#172
shaia merged 3 commits into
masterfrom
fix/grid-convergence-strict-monotonicity

Conversation

@shaia
Copy link
Copy Markdown
Owner

@shaia shaia commented Mar 6, 2026

Summary

The projection solver was already switched from Red-Black SOR to CG, which resolved non-monotonic grid convergence (17x17: 0.046, 25x25: 0.037, 33x33: 0.032). Remove the +0.02 tolerance slack from grid convergence assertions, require strict error decrease with refinement, and add convergence rate reporting.

Test plan

  • Grid convergence tests pass with strict monotonicity assertions
  • Convergence rate reporting added to test output
  • All existing validation tests still pass

🤖 Generated with Claude Code

… solver is used

The projection solver was already switched from Red-Black SOR to CG,
which resolved non-monotonic grid convergence (17x17: 0.046, 25x25:
0.037, 33x33: 0.032). Remove the +0.02 tolerance slack from grid
convergence assertions, require strict error decrease with refinement,
and add convergence rate reporting. Mark ROADMAP items as resolved.
Copy link
Copy Markdown
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

This PR tightens the grid convergence assertions in two validation test files for the lid-driven cavity solver. Now that the projection solver uses a CG Poisson solver (which produces monotonically decreasing errors with refinement), the previous tolerance slack (+0.02) is removed and strict monotonicity is enforced. Convergence rate reporting is also added to one of the test files.

Changes:

  • Replace relaxed grid convergence assertions (<= prev + 0.02) with strict monotonicity checks (<) in both test_ghia_projection_cpu.c and test_cavity_reference.c
  • Add convergence rate computation and reporting after the grid convergence loop in test_cavity_reference.c
  • Remove the now-unused prev_error variable in test_cavity_reference.c (errors are compared via the existing errors[] array)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tests/validation/test_ghia_projection_cpu.c Enforce strict error decrease with grid refinement using if (i > 0) guard instead of initializing prev_rms to a large value with tolerance slack
tests/validation/test_cavity_reference.c Remove prev_error variable, enforce strict monotonicity via errors[] array, tighten final finest-grid assertion, and add convergence rate reporting

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread tests/validation/test_cavity_reference.c
Comment thread tests/validation/test_ghia_projection_cpu.c Outdated
shaia added 2 commits March 6, 2026 09:44
CFD_ENABLE_OPENMP was never propagated to test binaries (only defined
in lib/ directory scope), so the compile-time #ifndef guard always
triggered, causing premature test skips and a TSan SEGV during exit.
The runtime poisson_solver_backend_available() check is sufficient.
Store errors in an array (matching test_cavity_reference.c pattern)
and report convergence rates after the grid refinement loop for
consistent diagnostic output across both grid convergence tests.
Copy link
Copy Markdown
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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@shaia shaia merged commit cee6b69 into master Mar 6, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants