mpi: implement MPI_Get_hw_resource_info version two - #14317
Merged
hppritcha merged 2 commits intoAug 26, 2026
Conversation
hppritcha
added a commit
to hppritcha/ompi
that referenced
this pull request
Aug 13, 2026
Add guidance explaining when it is (and is not) safe to annotate a symbol with OMPI_HIDDEN, so that contributors -- and AI coding agents in particular -- do not reintroduce the class of build breakage that need to be fixed in the initial version of code present in PR open-mpi#14317. As of Open MPI v6.0 the MPI interface is split across libmpi (Open MPI ABI) and libmpi_abi (standard MPI ABI), both of which link against the internal libopen_mpi. Many ompi_* symbols are defined in libopen_mpi but used from the bindings compiled into libmpi and libmpi_abi (for example, predefined handle objects like ompi_mpi_comm_parent and helpers like ompi_comm_split_type_hw_guided_support). Marking such a symbol OMPI_HIDDEN prevents it from being exported from libopen_mpi, so the links of both libmpi and libmpi_abi fail with unresolved symbols. Two files are updated: - AGENTS.md: add a "Golden rules" bullet describing the hazard and the rule of thumb -- if a symbol crosses a library boundary, use OMPI_DECLSPEC (or leave it un-annotated), never OMPI_HIDDEN. Only hide symbols that are certainly private to a single DSO. - docs/developers/source-code.rst: add a "Hiding symbols with OMPI_HIDDEN" subsection under Symbol Visibility that documents the v6.0 library structure and includes a warning admonition covering the unresolved-symbol failure mode and the same rule of thumb. These are documentation-only changes. CLAUDE.md is a symlink to AGENTS.md and is covered automatically. Signed-off-by: Howard Pritchard <howardp@lanl.gov>
hppritcha
force-pushed
the
masaqif-feature/hw-resource-info
branch
from
August 17, 2026 15:38
98ae066 to
46c694b
Compare
devreal
requested changes
Aug 17, 2026
Member
Author
|
@masaqif thanks for the contribution. are you affiliated with a university or other org? I am asking because for our copyright notice we prefer if a contributor is associated with an organization of some sort. There are a few exceptions but these have typically been for people who have been long contributing members but are no longer affiliated with a particular organization. |
Member
Author
|
its been decided that copyrights like those used in this PR are okay. |
Member
Author
|
hmm ci giving problems. close again. |
Populate URI-form hardware resource information from hwloc and accept the returned keys in hardware-guided communicator splitting. address hardware resource review feedback Serialize lazy topology discovery, use live CPU bindings for URI-guided splits, and return empty info outside the active MPI lifetime. Move and expand the regression test under test/mpi/environment and document communicator ownership and exclusion semantics. Signed-off-by: Musawer Ahmad Saqif <saqif@umich.edu> Signed-off-by: Howard Pritchard <howardp@lanl.gov>
hppritcha
force-pushed
the
masaqif-feature/hw-resource-info
branch
from
August 26, 2026 17:11
04369ac to
adaa89c
Compare
Member
Author
|
@devreal check now |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements hwloc-backed support for MPI_Get_hw_resource_info() in the OMPI layer and extends hardware-/resource-guided MPI_Comm_split_type() to accept hwloc://... URI resource keys (in addition to legacy Open MPI-specific strings), with accompanying unit test coverage and documentation/release-note updates.
Changes:
- Populate
MPI_Get_hw_resource_info()with per-resource boolean keys based on live hwloc binding/topology data. - Extend
MPI_Comm_split_type()parsing and splitting logic to support URI-formmpi_hw_resource_typevalues and perform binding-aware grouping/exclusion. - Add a new OMPI-level unit test and update man pages + v6.1.x changelog to describe the new behavior.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
ompi/test/general/Makefile.am |
Adds the new hw_resource_info unit test to make check. |
ompi/test/general/hw_resource_info.c |
New singleton test covering lifetime rules, threading, and URI-guided split behavior. |
ompi/mpi/c/get_hw_resource_info.c.in |
Implements MPI_Get_hw_resource_info() using hwloc-derived restrictions. |
ompi/communicator/Makefile.am |
Installs/exports the new communicator split-type helper header. |
ompi/communicator/comm.c |
Adds hwloc-guided split path and exports shared resource metadata for reuse. |
ompi/communicator/comm_split_type.h |
New shared declarations for hwloc-guided split helpers and resource metadata. |
docs/release-notes/changelog/v6.1.x.rst |
Notes the new hwloc-backed behavior and URI key support. |
docs/man-openmpi/man3/MPI_Get_hw_resource_info.3.rst |
Updates the man page with semantics, keys, and examples. |
docs/man-openmpi/man3/MPI_Comm_split_type.3.rst |
Documents accepted URI values for guided splits. |
.gitignore |
Attempts to ignore the new test binary (currently points to the wrong path). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
devreal
previously approved these changes
Aug 26, 2026
devreal
left a comment
devreal
left a comment
Contributor
There was a problem hiding this comment.
Not sure about OMPI_DECLSPEC suggestion. Otherwise looks good to me.
Signed-off-by: Howard Pritchard <howardp@lanl.gov> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Howard Pritchard <hppritcha@gmail.com>
Member
Author
|
i think the OMPI_DECLSPEC is a good idea. I wonder if co-pilot saw the documentation added in #14318 . |
Member
Author
|
Could you re-review? docs builds are acting weird today. it keeps getting canceled for some reason. |
devreal
approved these changes
Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Populate URI-form hardware resource information from hwloc and accept the returned keys in hardware-guided communicator splitting.
Signed-off-by: Musawer Ahmad Saqif saqif@umich.edu