Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new hierarchical-parallel helper to search an inner index range in parallel, intended to support upcoming physics (e.g., submesoscale eddy parameterization) by enabling “find first match” behavior inside team-level kernels.
Changes:
- Added
parallelSearchInnerto the Kokkos hierarchical-parallel wrappers (returns first index matching a predicate, or-1). - Added a unit test covering full-range and sub-range search behavior.
- Updated the developer guide to document the new inner-loop pattern.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| components/omega/src/infra/OmegaKokkos.h | Implements parallelSearchInner with host/device-specific behavior. |
| components/omega/test/infra/OmegaKokkosHiParTest.cpp | Adds coverage validating correctness of the new search primitive. |
| components/omega/doc/devGuide/ParallelLoops.md | Documents parallelSearchInner and provides an example usage snippet. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| KOKKOS_FUNCTION void parallelSearchInner(const TeamMember &Team, int UpperBound, | ||
| F &&Functor, int &Idx) { | ||
| static_assert(std::is_same_v<std::invoke_result_t<F, int>, bool>, | ||
| "paralleSearchInner requires a functor that takes an int and " |
There was a problem hiding this comment.
The static_assert message misspells the API name as paralleSearchInner, which makes diagnostics harder to search for. Please correct it to parallelSearchInner.
| "paralleSearchInner requires a functor that takes an int and " | |
| "parallelSearchInner requires a functor that takes an int and " |
| int Idx; | ||
| parallelSearchInner(Team, N2, INNER_LAMBDA(Int J2) { | ||
| return M(J1, J2) > Threshold; | ||
| }, Idx); |
There was a problem hiding this comment.
In the new parallelSearchInner documentation example, the inner lambda parameter type is written as Int (capital I), but it should be int to match the actual API usage and avoid a non-compiling snippet.
| To search an index range in parallel for the first index where a given condition occurs Omega | ||
| provides the `parallelSearchInner` function. |
There was a problem hiding this comment.
This sentence reads as though a word is missing: “...where a given condition occurs Omega provides...”. Adding a comma after “occurs” (or rephrasing) would fix the grammar.
| To search an index range in parallel for the first index where a given condition occurs Omega | |
| provides the `parallelSearchInner` function. | |
| To search an index range in parallel for the first index at which a given condition occurs, | |
| Omega provides the `parallelSearchInner` function. |
Adds a function for searching an index range in parallel at the inner level of hierarchical parallelism. It is going to be useful in implementing the submesoscale eddy parametrization.
Checklist
Testingwith the following:have been run on and indicate that are all passing.
has passed, using the Polaris
e3sm_submodules/Omegabaseline-pfor both the baseline (Polarise3sm_submodules/Omega) and the PR build