Skip to content

Fix LINELET with mpi, add regression case for laminar NACA0012 - #2899

Merged
bigfooted merged 8 commits into
su2code:developfrom
Syphonicc:linelet-regression-test
Oct 12, 2026
Merged

bigfooted merged 8 commits into
su2code:developfrom
Syphonicc:linelet-regression-test

Conversation

@Syphonicc

Copy link
Copy Markdown

Addresses the regression-test part of #2877.

Adds a serial regression case for LINEAR_SOLVER_PREC= LINELET. There is currently no LINELET coverage in TestCases.

TestCases/navierstokes/naca0012/lam_NACA0012_LINELET.cfg is the laminar NACA0012 config with one added line, LINEAR_SOLVER_PREC= LINELET. The baseline uses the default ILU(0).

Note: it is based on the updated lam_NACA0012.cfg from #2866 (ITER instead of the deprecated EXT_ITER, CFL 30), so on current develop the two files differ in more than that one line until #2866 is merged. This PR does not depend on #2866; the new file parses on develop as-is. The registration is placed after cylinder_lowmach, away from the lam_naca0012 entry added in #2866, so the two PRs don't touch the same lines.

At test_iter = 20 the two cases already differ, so the test is sensitive to the preconditioner:

Preconditioner rms[Rho] rms[RhoU] rms[RhoV] rms[RhoE]
ILU(0) (lam_NACA0012) -5.522140 -3.127430 -3.286926 -0.109971
LINELET -5.282867 -2.845208 -2.852421 0.154542

test_vals are from a local serial clean build of current develop (8118ee0, GCC 15.2.1). The same procedure reproduces the #2866 lam_naca0012 values exactly. I'll update them if CI differs.

This does not address the segfault reports in #2877, and only covers the serial path.

Same as lam_NACA0012.cfg with LINEAR_SOLVER_PREC= LINELET, compared against the default ILU(0). See su2code#2877.
@bigfooted

Copy link
Copy Markdown
Contributor

can you check parallel?

@Syphonicc

Copy link
Copy Markdown
Author

Good catch LINELET segfaulted in parallel with ≥4 ranks (1 and 2 ranks ran fine, which is why it didn't show up before).

Cause

  • Partitions without wall markers have no linelets, so CSysMatrix::BuildLineletPreconditioner never allocated the per-thread working vectors but still indexed them → segfault (CSysMatrix.cpp, confirmed with gdb on the rank that had no walls).
  • CGeometry::GetLineletInfo used linelets.empty() as its "already computed" check. On ranks with no linelets it recomputed on every call, including the MPI_Allreduce/MPI_Allgather, while the other ranks returned early → mismatched collectives.

Fix

  • Always allocate the linelet working vectors (they're small, MAX_LINELET_POINTS per thread).
  • Added an explicit computed flag to CLineletInfo so every rank computes it exactly once.
  • Skip the "points in each line" message when there are no linelets globally (avoids a divide by zero).

Tests

  • Added lam_naca0012_linelet to parallel_regression.py with mpirun -n 4. The default 2 ranks doesn't trigger the bug, because both partitions touch the airfoil.
  • Merged develop and updated the serial test_vals.
  • Checked the case runs at 1, 2, 4 and 8 ranks.

Fixes #2877.

@rois1995 rois1995 self-assigned this Oct 1, 2026

@rois1995 rois1995 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks! Tested: on develop this case crashes on 4 and 8 ranks; with this PR it runs on 2, 4 and 8.

Comment thread Common/src/geometry/CGeometry.cpp
Comment thread TestCases/parallel_regression.py Outdated
Comment thread TestCases/navierstokes/naca0012/lam_NACA0012_LINELET.cfg Outdated
@Syphonicc

Copy link
Copy Markdown
Author

Thanks for testing! CI hasn't actually run yet: all workflow runs on this PR show action_required, so they need a maintainer to approve them. On slots: the CI image uses Open MPI, which counts physical cores by default, so I've added --oversubscribe to the 4-rank command to be safe (the harness already relies on the Open MPI-specific --allow-run-as-root). Header updated as well.

@bigfooted bigfooted changed the title Add LINELET regression case for laminar NACA0012 Fix LINELET with mpi, add regression case for laminar NACA0012 Oct 2, 2026
@Syphonicc

Copy link
Copy Markdown
Author

CI findings:

ASAN (serial): a real, pre-existing bug that this PR's serial LINELET test exposed. In GetLineletInfo, nColors is su2uint (uint32_t) but is sent with MPI_UNSIGNED_LONG in the Allgather, so 4 bytes past the variable are read (stack-buffer-overflow in the no-MPI wrapper, and garbage colour offsets with real MPI). Fixed by declaring it unsigned long. Verified with an ASAN build: clean, and the test values are unchanged.

ramp_msw (parallel): looks unrelated. It uses ILU, so LINELET is never called, and it passes locally with residual deltas of about 0.09 against its 0.2 tolerance. It seems to be a noisy test.

@bigfooted
bigfooted merged commit 7c18ea5 into su2code:develop Oct 12, 2026
38 of 39 checks passed
@pragyaangaur

Copy link
Copy Markdown

Nice fix. Testing at 4 ranks was the key insight, since at 2 ranks every partition touches the airfoil and hides the crash. One suggestion for a follow-up: the test only covers the fix if the partitioner leaves a rank with no wall nodes, so a small assertion or log of the zero-linelet rank count would keep it from losing that coverage if the partitioning changes.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants