Skip to content

build(c++): report machine flags used - #211

Merged
Panadestein merged 13 commits into
refactor-use-hwlocfrom
build-report-machine-flags-used
Aug 13, 2026
Merged

build(c++): report machine flags used#211
Panadestein merged 13 commits into
refactor-use-hwlocfrom
build-report-machine-flags-used

Conversation

@robertodr

Copy link
Copy Markdown
Member

Summary

🤖 AI text below 🤖

This pull request introduces reporting of machine-dependent compiler flags (such as those set via -march) used to build the project. It adds a mechanism to query, clean, and expose the actual flags used for the active variant, making this information available at both the C++ and Python levels. Additionally, it introduces a utility script and its tests for parsing GCC target options.

Machine-dependent flags reporting and variant introspection:

  • Added a CMake function (_monoprop_query_machine_flags) to query and clean the machine-dependent flags used for each build variant, storing them in CMake variables and generating a new header (Variants.h) with compile-time functions to report the active variant and flags. [1] [2]
  • Updated Info.h.in and the C++ bindings to expose the active variant and its compiler flags via variant() and variant_flags() functions, which are now also accessible in Python as monoprop.__variant__ and monoprop.__compiler_flags__. [1] [2] [3] [4] [5] [6]
  • Updated the test workflow to print the variant and compiler flags in CI logs for easier debugging and reproducibility.

Utility script and testing:

  • Added tools/gcc-target-help-clean.py, a script to normalize the output of gcc -Q --help=target, and a corresponding test suite to ensure robust parsing and cleaning of compiler flag output. [1] [2]

CMake and source organization:

  • Ensured generated headers (VariantMacros.h, Info.h, etc.) are included in the public interface and properly listed in the CMake configuration.
  • Clarified and simplified CMake comments and logic regarding module scanning and flag handling.

These changes improve transparency into the build environment, facilitate reproducibility, and make it easier to debug and optimize builds for different architectures.

Checklist

  • Tests added or updated to cover the changes
  • Documentation updated (docstrings, docs/, CONTRIBUTING.md) if needed
  • CHANGELOG / release notes updated if applicable

AI/LLM disclosure

  • I did not use LLM tooling, or used it only privately for ideation
  • I used the following tool to help write this PR description:
  • I used the following tool to generate or modify code: GitHub Copilot, claude-opus-4.8

Important

By opening this PR I confirm that I have read CONTRIBUTING.md and I agree to the terms of the Contributor License Agreement.

Warning

If you're contributing on behalf of your employer, contact cla@algorithmiq.fi to arrange a Corporate CLA.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Docs preview: https://pr-211.monoprop-docs.pages.dev

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.70%. Comparing base (3fc31ae) to head (710790e).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@                 Coverage Diff                 @@
##           refactor-use-hwloc     #211   +/-   ##
===================================================
  Coverage               97.70%   97.70%           
===================================================
  Files                      14       14           
  Lines                     742      742           
  Branches                   98       98           
===================================================
  Hits                      725      725           
  Misses                     12       12           
  Partials                    5        5           
Flag Coverage Δ
cpp 97.70% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@robertodr
robertodr force-pushed the build-report-machine-flags-used branch from fd4f296 to e0da3d3 Compare August 7, 2026 12:28
@github-actions github-actions Bot added the dependencies Pull requests that update a dependency file label Aug 10, 2026
@robertodr
robertodr force-pushed the build-report-machine-flags-used branch from 8a2789c to 45738c6 Compare August 10, 2026 08:40
Comment thread tests/test_target_help_clean.py
ludmilaasb
ludmilaasb previously approved these changes Aug 10, 2026

@ludmilaasb ludmilaasb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@robertodr
robertodr dismissed ludmilaasb’s stale review August 10, 2026 18:51

The merge-base changed after approval.

@robertodr
robertodr force-pushed the build-report-machine-flags-used branch from 8729620 to 7edf3cd Compare August 10, 2026 18:51
@robertodr
robertodr force-pushed the build-report-machine-flags-used branch 2 times, most recently from 8e8111f to 6a207bb Compare August 11, 2026 09:33
@robertodr
robertodr force-pushed the build-report-machine-flags-used branch from 6a207bb to 0129dbd Compare August 13, 2026 09:01
@robertodr
robertodr force-pushed the build-report-machine-flags-used branch from 0129dbd to 984742c Compare August 13, 2026 09:44
@robertodr
robertodr force-pushed the build-report-machine-flags-used branch from 984742c to 20e1219 Compare August 13, 2026 09:57
@robertodr
robertodr force-pushed the build-report-machine-flags-used branch from 20e1219 to 2cc7c5b Compare August 13, 2026 10:20
@robertodr
robertodr force-pushed the build-report-machine-flags-used branch from 2cc7c5b to 710790e Compare August 13, 2026 10:23
@sonarqubecloud

Copy link
Copy Markdown

@Panadestein
Panadestein merged commit 1bd8dc7 into main Aug 13, 2026
32 of 33 checks passed
@Panadestein
Panadestein deleted the build-report-machine-flags-used branch August 13, 2026 10:40
diagonal-hamiltonian added a commit that referenced this pull request Aug 13, 2026
Brings in #203 (implementations split into source/inline files), #208 (hwloc
replaces the custom topology discovery), #211, #223, #148 and #224. Resolves the
conflict that had left this PR with no CI: GitHub cannot recompute
refs/pull/166/merge while the merge conflicts, so the pull_request workflows
produced no runs at all and only the pull_request_target labeller was firing.

Four files conflicted; the rest auto-merged. Two were unions of independent
additions (AGENTS.md, parallelism.mdx) -- in the latter, main's wording for the
monoprop_PARTITION_PINNING row is now the accurate one, since hwloc replaced the
Linux-only /sys path.

The substantive resolution is CpuTopology. #208 rewrote discovery onto hwloc,
which is the better mechanism and is kept wholesale, but it also reinstated the
placement bug this branch exists to fix: enumerate_physical_cores() filters by
the calling thread's affinity, and placement_order() then rejects the request
when group_count * n > cores.size(), so a rank holding a launcher-assigned slice
divides an already-divided machine and silently runs unpinned. That is the
mechanism measured at 437 us/sync against 15.5 us/sync placed.

So this keeps main's hwloc discovery and re-applies the fix on top of it, rather
than keeping either side's file:

  - NodeMask, classify_node_mask() and this_thread_cpumask() return, with the
    PerRank collapse moved into partition_cpusets() so placement_order() stays
    pure and hardware-free the way #208 factored it.
  - CpuMask replaces cpu_set_t as the exchanged type, since main's CpuSet is now
    a single PU rather than a mask. It is a fixed-size POD because
    PartitionGroup ships it through MPI_Allgather as MPI_BYTE, and being
    hwloc-free makes classify_node_mask() unit-testable without live hardware.
    4096 bits rather than glibc's CPU_SETSIZE of 1024: a PU index past the mask
    is invisible to the disjointness test, which would misread a per-rank split
    as shared.
  - pin_this_thread() reports whether the affinity took, which CommProfile's
    pinned count needs to tell "nothing was pinned" apart from "one domain per
    rank" when barrier_groups is 0.
  - cpuset_domains() returns, so the two-level barrier keeps deriving its
    domains from the placement rather than from the placement logic.

Verified on a Deucalion login node, against live hwloc and real launcher
bindings, that the collapse is load-bearing and does not disturb the shared
case. Same live mask, same call, only the classification differs:

  2 ranks, --map-by slot:PE=4 --bind-to core, n=4  =>  PerRank
      as Shared (== main's unconditional divide): 0 cores placed
      as PerRank:                                 4 cores placed
  2 ranks, PE=8, n=8                              =>  PerRank:  0 vs 8
  2 ranks, --bind-to none, n=4                    =>  Shared:   4 vs 4

AGENTS.md records the trap under Architecture Overview, including that it has now
regressed once, because the guard lives in the placement policy and not in
discovery -- so a future rework of that layer has to re-check it.
cpu_topology_policy_per_rank_slice_starves_without_collapse pins the mechanism
deterministically, without live hardware.

Gates on the merged tree (hwloc 2.12.1, already provided transitively by
foss/2025b, so hpc/deucalion/env.sh needs no change):
  - ctest -L serial: 224/224 passed, 0 failed, 412 s
  - fabric-exclusion scoping still correct: 224/224 serial carry it, 0/1 mpi do
  - cpu_topology: 14/14 cases, 185 assertions, with per-case assertion counts
    confirming the two live restricted-mask cases asserted rather than
    early-returning
  - rank_count_matches_under_an_asymmetric_emit_gate and
    zero_cutoff_upper_atol_zero_is_exact_World: pass at 1, 2 and 4 ranks

Unchanged by this merge and still open: shm_comm_oversubscribed_repeated_collectives
aborts under 2 ranks on main as well as here, so ctest -L mpi remains unusable as
a gate; and the layout A build_graph ~1.1x regression is still unexplained.

Assisted-by: ClaudeCode:claude-opus-5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci cpp dependencies Pull requests that update a dependency file python tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants