Flex ranks: coherence read-outcome diagnostic (#742) - #816
Merged
DLWoodruff merged 6 commits intoAug 10, 2026
Conversation
…outcome diagnostic (Pyomo#742) Replace the unequal-rank path's interim fullcomm.allgather buffer-layout exchange with a scalable two-level scheme (allgather within each cylinder, allgather across one anchor rank per cylinder, broadcast within each cylinder), chosen over local-compute because extensions register send fields dynamically. Localized to how strata_buffer_layouts is populated at startup; the multi-source reader is untouched, and the equal-rank path keeps its strata_comm allgather verbatim. Add an always-on per-field read-outcome counter to the unequal-rank multi-source reader (new_accepted / not_new / rejected_incoherent / rejected_cross_reader / accepted_mixed), so an infrequently-reporting bounds cylinder can be diagnosed as a coherence problem vs. a slow upstream sender. Each cylinder prints a per-field summary at finalization (aggregated across its ranks, rank-0-gated, only for fields that did multi-source reads); an opt-in periodic line (coherence_diagnostics_period in the spcomm options) supports live debugging; counters are exposed as SPCommunicator.coherence_counters. Tests: new test_flex_layout_exchange.py pins the two-level exchange against fullcomm.allgather across several cylinder partitions (wired into CI and run_coverage.bash); the deterministic coherence-policy stub now also pins the counter buckets; test_flexible_rank_duals.py asserts the counters partition the total and that the strict DUALS field never accepts a blended read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #816 +/- ##
==========================================
+ Coverage 76.79% 76.83% +0.04%
==========================================
Files 177 177
Lines 23456 23498 +42
==========================================
+ Hits 18012 18054 +42
Misses 5444 5444 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Local-compute of remote layouts is not impossible in principle; the real obstacle is that the library has no static declaration surface for fields (layouts arise only from runtime register_send_field calls, and custom drivers need not use Config), so it would require a new mandatory declare-your-fields API plus re-deriving remote scenario slices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The scaling analysis does not support replacing the flat fullcomm.allgather layout exchange: it runs once at startup, allgather latency is O(log N), and the O(N) per-rank data is the result itself (every rank needs all N layouts), so the two-level scheme has the same asymptotics and only changes the collective's participant pattern -- a constant-factor effect on a cold path, at even 10,000 ranks a few MB per rank against hours of solves. Pyomo#726 will be closed won't-fix; the design doc's Option D note, Phase-2 bullet, and release-gate list now record that rationale. The coherence read-outcome diagnostic (Pyomo#742) is unchanged and remains this branch's content. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DLWoodruff
marked this pull request as ready for review
August 8, 2026 01:36
Two review findings on the read-outcome diagnostic. `coherence_diagnostics_period` could never take effect on a bound spoke -- the cylinders whose infrequent reporting the diagnostic exists to explain. `_BoundSpoke.__init__` and `InnerBoundSpoke.__init__` take no `communicators` parameter, so the cylinder list WheelSpinner passes positionally binds to their `options` parameter and is forwarded into `communicators`, leaving `SPCommunicator.options` empty on every spoke (this diagnostic was its first consumer, so nothing had noticed). Read the period from `opt.options` instead, which is set from `opt_kwargs` for every cylinder and already houses the sibling cylinder-wide debug switches `trace_prefix` and `inspect_buffers_on_shutdown`. The printed `miss rate` counted only `rejected_incoherent` and `accepted_mixed`. One straddled publish on an R-rank reader records one `rejected_incoherent` and R-1 `rejected_cross_reader`, so the reported rate was the truth divided by the reader-rank count (a 2-rank spoke showed 2.58% where 5.25% of read attempts had been rejected). The rate now comes from a `coherence_miss_rate` helper that counts all three non-clean outcomes -- equivalently, every read that was neither a clean accept nor a clean nothing-to-take. Relatedly, the two rejection buckets now split on whether this rank's own sources disagreed rather than on the field's coherence policy. A relaxed field can straddle a publish too: its floor then differs from a peer reader's and the collective check rejects. That is this rank's coherence miss, but `rejected_incoherent` was gated on `strict and mixed` and so was unreachable for every relaxed field, which sent those misses to `rejected_cross_reader` in contradiction of that bucket's documented meaning. Tests: a stub cylinder_comm lets the serial reader tests drive the collective cross-reader check, pinning both the relaxed-field local straddle and the peer-caused rejection to their buckets; the miss-rate formula is pinned directly. Both new assertions fail against the old behavior. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Coherence read-outcome diagnostic for the unequal-rank (flexible-ranks) path, localized to the multi-source reader.
Closes #742.
(This PR originally also replaced the unequal-rank layout exchange with a two-level scheme (#726). That change has been dropped: the flat
fullcomm.allgatherruns once at startup, its latency is O(log N), and its O(N) per-rank data volume is the result itself — every rank needs every peer's layout, so any replacement still delivers O(N) data per rank and only changes the collective's participant pattern. At even 10,000 total ranks the flat exchange moves a few MB per rank, milliseconds against hours of solves. #726 is closed won't-fix with that rationale, and the design doc's Option D note, Phase-2 bullet, and release-gate list now record it.)Coherence read-outcome diagnostic (#742)
The unequal-rank multi-source reader now counts every multi-source read (>= 2 sources; a single source cannot straddle a publish) into one of five buckets at the
reduce_source_write_idscall site:new_accepted— coherent, advanced, usednot_new— coherent, but the sender has not published (slow sender)rejected_incoherent— this rank's sources disagreed (the fundamental coherence miss: the read straddled a publish)rejected_cross_reader— this rank's sources agreed, the collective cross-reader check rejected (typically the shadow of some other reader rank's straddle)accepted_mixed— relaxed field, blended assembly usedThe reported
miss rateis every read a straddled publish cost —rejected_incoherent+rejected_cross_reader+accepted_mixed, equivalently everything that was neither a clean accept nor a clean nothing-to-take. Counting only the locally-detected misses would divide the rate by the reader-rank count, since one straddle on an R-rank reader records onerejected_incoherentand R-1rejected_cross_reader.so an infrequently-reporting bounds cylinder can be diagnosed as coherence problem (
rejected_incoherent/accepted_mixeddominate) vs. slow upstream sender (not_newdominates). Counting is always-on (two integer increments per multi-source read); each cylinder prints a per-field summary at finalization (aggregated across its ranks with one gather, rank-0-gated, only for fields that did multi-source reads — equal-rank runs print nothing). An opt-in periodic line (coherence_diagnostics_periodin the cylinder'sopt_kwargsoptions) supports live debugging, and the counters are exposed asSPCommunicator.coherence_counters. Sample from the farmer 4+2 integration test:Tests
test_flex_coherence_policy.py: the deterministic stub reader now also pins the counter buckets (strict mixed →rejected_incoherent, relaxed mixed →accepted_mixed, coherent no-advance →not_new, buckets partition the total).test_flexible_rank_duals.py: after both split directions, asserts the buckets partition the total, that the strictDUALSfield never accepts a blended read, and that the 4+2 bound came from accepted multi-source reads (in 2+4 the spoke's slices can nest inside the hub's halves, leaving every read single-source, so counter presence is only guaranteed in 4+2). All existing flex MPI tests (cylinders,duals,xhat,xfeas,xhat_multistage,extension_fields,spwindow_multisource) pass locally at -np 6.The design doc moves the #742 diagnostic from "possible future work" into the coherence section.
Deliberately not in this PR: #749 (reduced_costs spoke at flexible ratios, Phase 7) — behavioral work on the spoke itself, better reviewed separately.
🤖 Generated with Claude Code