Skip to content

Conversation

@shbhmexe
Copy link
Contributor

@shbhmexe shbhmexe commented Jan 11, 2026

This PR fixes logic bugs in the 2D Actuator Disk surface splitting code in CSU2ASCIIMeshReaderFVM.cpp:

  1. Fix wrong loop variable: Changed ActDiskPoint_Front[iEdge] to ActDiskPoint_Front[iPoint] - was using wrong variable causing incorrect perimeter detection
  2. Fix wrong array access: Changed EdgeBegin[...] to ActDiskPoint_Front[...] - was indexing wrong array causing potential segfault
  3. Remove incorrect unique() call: The perimeter detection needs frequency information which unique() was destroying

Related Work

No related PRs or issues.

PR Checklist

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

- CSU2ASCIIMeshReaderFVM.cpp: Fix uniqueness logic and array indexing in 2D Actuator Disk splitting.
- CSolver.cpp: Fix matrix corruption in Periodic Least Squares (replace duplicate r23_b with 0.0).

Signed-off-by: shbhmexe <[email protected]>
@shbhmexe shbhmexe changed the base branch from master to develop January 11, 2026 09:59
- CSU2ASCIIMeshReaderFVM.cpp: Fix uniqueness logic and array indexing in 2D Actuator Disk splitting.

- CSolver.cpp: Fix matrix corruption in Periodic Least Squares (replace duplicate r23_b with 0.0).

Signed-off-by: shbhmexe <[email protected]>
shbhmexe and others added 4 commits January 12, 2026 13:58
- Restore CSolver.cpp to original state (r23_b changes were incorrect)

- Keep CSU2ASCIIMeshReaderFVM.cpp fixes: unique() removal and iEdge->iPoint fix

Signed-off-by: shbhmexe <[email protected]>
Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

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

Do you have before/after results?

@shbhmexe
Copy link
Contributor Author

Do you have before/after results?

No before/after numerical results available as this fixes 2D Actuator Disk code path which has no regression test coverage.

The changes are logical corrections:

Fix wrong variable iEdge → iPoint in loop
Fix wrong array EdgeBegin → ActDiskPoint_Front
Remove unique() that was destroying frequency info needed for perimeter detection
These would cause incorrect mesh splitting or segfaults for 2D actuator disk cases.

@shbhmexe
Copy link
Contributor Author

and also check this pr #2669 and #2663

@pcarruscag
Copy link
Member

You'll need to add a regression test for this.

- Added simple 2D mesh (actdisk_2d.su2) with 2-edge actuator disk marker to reproduce perimeter detection issues.

- Added dry-run configuration (config.cfg).

- Registered '2d_actuator_disk_check' in serial_regression.py.
This will be used to do checks when code is pushed to github
to make sure nothing is broken. '''

args = parse_args('Parallel Regression Tests')

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable args is not used.
This will be used to do checks when code is pushed to github
to make sure nothing is broken. '''

args = parse_args('Parallel Regression AD Tests')

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable args is not used.
@shbhmexe
Copy link
Contributor Author

You'll need to add a regression test for this.

i add regression test for this particular fix

@bigfooted
Copy link
Contributor

You'll need to add a regression test for this.

i add regression test for this particular fix

The su2 mesh goes into the repository su2code/Testcases
you can then test the regressions by modifying .github/workflow/regression.yml, change -c develop into -c fix_actuator_disk or whatever the branch in Testcases is called:

      # -t <Tutorials-branch> -c <Testcases-branch>
      args: -b ${{github.ref}} -t develop -c develop -s ${{matrix.testscript}}

@shbhmexe
Copy link
Contributor Author

You'll need to add a regression test for this.

i add regression test for this particular fix

The su2 mesh goes into the repository su2code/Testcases you can then test the regressions by modifying .github/workflow/regression.yml, change -c develop into -c fix_actuator_disk or whatever the branch in Testcases is called:

      # -t <Tutorials-branch> -c <Testcases-branch>
      args: -b ${{github.ref}} -t develop -c develop -s ${{matrix.testscript}}

Done! I've:

Added test registration in serial_regression.py
Updated regression.yml to use -c fix_actuator_disk Will push mesh files to su2code/TestCases on fix_actuator_disk branch

@shbhmexe shbhmexe force-pushed the fix/2d-actuator-disk-and-periodic-solver-bugs branch from fa1c111 to c9401a1 Compare January 17, 2026 07:23
@shbhmexe shbhmexe force-pushed the fix/2d-actuator-disk-and-periodic-solver-bugs branch from a2e3890 to ba35713 Compare January 17, 2026 07:26
shbhmexe added a commit to shbhmexe/TestCases that referenced this pull request Jan 17, 2026
- actdisk_2d.su2: 2D mesh with actuator disk boundary

- config.cfg: Dry-run configuration for testing

Related: su2code/SU2#2694
- Reverted regression.yml to use -c develop

- Removed 2d_actuator_disk_check test until TestCases PR is merged
@shbhmexe shbhmexe force-pushed the fix/2d-actuator-disk-and-periodic-solver-bugs branch from b914248 to 1971a64 Compare January 17, 2026 08:52
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.

3 participants