Skip to content

Conversation

@guptapratykshh
Copy link

@guptapratykshh guptapratykshh commented Jan 4, 2026

Proposed Changes

This PR addresses the performance bottleneck experienced when using a large number of probes (e.g., >100) in parallel simulations. The previous implementation used a brute-force O(N) linear search per probe to find the nearest grid point, resulting in O(N_probes * N_points) complexity which caused significant slowdowns.

The changes include:

  1. ADT-Based Nearest Neighbor Search: Implemented an Alternating Digital Tree (ADT) strategy for probe location. This reduces search complexity to O(log(N_points)) per probe. A heuristic is used to switch to ADT only when the number of probes exceeds a threshold (default: 10).
  2. Batched Communication: Consolidated the AllReduce operations for probe values. Instead of performing one MPI reduction per probe, all probe values are now collected and reduced in a single batched operation at the end of the output routine, significantly reducing MPI overhead.
  3. Regression Testing: Added a new regression test case (test_11_probes.cfg) to parallel_regression.py that specifically exercises the ADT path (11 probes) and verifies the correctness of the probe output against known values.

Related Work

Fixes issue #2546 (Probe performance bottleneck).

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.

@guptapratykshh
Copy link
Author

guptapratykshh commented Jan 4, 2026

Please look into this PR , while removing the formatting all the changes of the file (CFlowOutput.cpp) were removed by mistake in the last PR. @pcarruscag

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.

Thanks, looks good now.

@bigfooted
Copy link
Contributor

@guptapratykshh can you update the regression values in parallel_regression.py (after you've checked that everything works as intended)?

@pcarruscag
Copy link
Member

Are you using AI to review PRs @bigfooted?
Why would this PR change the probe value @guptapratykshh?!

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.

Unexplained regressions

@guptapratykshh
Copy link
Author

The values I initially committed for this new test case (probe_performance_11) were incorrect: the probe values were accidentally taken from Iteration 0 (initialization), while the RMS density was from the last iteration.

I updated parallel_regression.py to consistently check the values at the final iteration (Iter 3), ensuring the test actually validates the simulation result.

@pcarruscag
Copy link
Member

The regression values for test flatplate_udobj changed, this case only uses one probe, hence its results should not be affected by the ADT change. I suspect some kind of memory error or undefined behavior.
Please determine what is introducing the discrepancy.

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