Flex ranks: coherence read-outcome diagnostic (#742)

doc/designs/flexible_rank_assignments.md

@@ -367,34 +367,37 @@ window creation.  Cons:
 - Requires rewriting `SPCommunicator` and `SPWindow` to work without
   `strata_comm`.
 - The buffer layout exchange (currently `strata_comm.allgather`) needs
-  a replacement.  The cheapest option is to compute layouts locally on
-  every rank: per-cylinder rank count, the output of
-  `_calculate_scenario_ranks`, and field-registration order are all
-  static and deterministic at startup, so no communication is
-  required.  If a runtime exchange is genuinely needed (e.g., dynamic
-  field registration), prefer a two-level scheme —
-  `cylinder_comm.allgather` followed by a small cross-cylinder gather
-  over one anchor rank per cylinder — rather than a single
-  `fullcomm.allgather`, which scales worse at N=thousands.
-
-  *What the communication-layer cut actually ships, and the release
-  gate.*  The first cut uses a single `fullcomm.allgather` for the
-  unequal-rank layout exchange.  This is deliberate but interim: it is
-  effectively zero new code (the existing `SPWindow` exchange run on
-  `fullcomm` instead of `strata_comm`), it is a one-time *startup* cost
-  on a cold path — not the RMA hot path — and at development/test scale
-  (a handful of ranks) it is free.  It is **not** the end state.
-  Because total rank counts in the thousands are a real operating
-  regime here, the O(N) startup allgather and its O(N)-per-rank layout
-  storage must be replaced by the two-level (or local-compute) scheme
-  **before flexible ranks is documented or recommended for production
-  use** (it can land on `main` before then, since it is inert until a
-  non-default ratio).  That replacement is its own focused change — it
-  touches only how
-  `strata_buffer_layouts` is populated at startup, not the multi-source
-  reader — so it is tracked as a release-gate item rather than folded
-  into the feature phases, letting it be reviewed and scale-tested on
-  its own.  See §Gate reliance on the feature with an MPI CI matrix.
+  a replacement on the unequal-rank path, where the window comm is
+  `fullcomm`.
+
+  *What the unequal-rank path ships: the flat `fullcomm.allgather`,
+  permanently.*  The exchange is the existing `SPWindow` allgather run
+  on `fullcomm` instead of `strata_comm`.  This was first framed as
+  interim, with a "scalable" replacement (two-level or local-compute)
+  gating production use; that replacement was examined and rejected
+  (Pyomo/mpi-sppy#726, closed won't-fix).  The scaling analysis does
+  not support it: the exchange runs once, at startup, inside window
+  creation; an allgather's latency is O(log N) rounds in any production
+  MPI implementation, and its O(N) per-rank data volume is the *result*
+  — every rank legitimately needs the layout of any peer rank its
+  overlap maps touch, so any replacement scheme still delivers all N
+  layouts to every rank and cannot beat O(N) per-rank data.  Each
+  layout is a small dict of 3-int tuples; at even 10,000 total ranks
+  the flat exchange moves a few MB per rank — milliseconds, against a
+  startup that builds Pyomo scenario models and a run that solves
+  optimization problems for hours.  A two-level scheme (allgather
+  within each cylinder, allgather across one anchor rank per cylinder,
+  broadcast of the assembled table) changes only the collective's
+  participant pattern, not the asymptotics, and adds code to the
+  unequal-rank path for a constant-factor effect on a cold path.
+  Local-compute is further foreclosed structurally: the library has no
+  static declaration surface for fields — layouts exist only as the
+  side effect of runtime `register_send_field` calls made by cylinder
+  classes and extensions, and the cfg cannot serve as that surface
+  (drivers that build their own hub/spoke dicts need not use `Config`
+  at all) — so computing remote layouts locally would require a new
+  mandatory declare-your-fields API for cylinders, extensions, and
+  custom drivers, plus re-deriving each remote rank's scenario slice.
 
   *Lock granularity.*  `MPI_Win_lock(rank=target, ...)` is per-
   target-rank in the MPI spec, not per-window — a writer's exclusive
@@ -578,6 +581,41 @@ two code paths in the multi-source reader.  No cylinder-wide iteration
 counter (it would add synchronization the async design avoids and is
 unnecessary given the per-field analysis).
 
+#### Read-outcome diagnostic
+
+An always-on, per-field counter at the multi-source reader
+(Pyomo/mpi-sppy#742; `_count_coherence_read` in `spcommunicator.py`)
+buckets every multi-source read (>= 2 sources) as `new_accepted` /
+`not_new` / `rejected_incoherent` / `rejected_cross_reader` /
+`accepted_mixed`.  This lets an infrequently-reporting bounds cylinder
+be diagnosed as a coherence problem (reads rejected because sources
+disagree on `write_id`, or blended on a relaxed field) vs. a slow
+upstream sender (no new data), and measures how often a multi-source
+read actually straddles a publish — the empirical basis for the
+strict-vs-relaxed choices above, especially under an asynchronous APH
+sender.  Cost is two integer increments per multi-source read, so
+counting is unconditional; each cylinder prints a per-field summary at
+finalization (`report_coherence_diagnostics`, aggregated across the
+cylinder's ranks, rank-0-gated, only for fields that did multi-source
+reads — so equal-rank runs print nothing), and an opt-in periodic line
+(`coherence_diagnostics_period` in the cylinder's `opt_kwargs` options:
+print local counters every N multi-source reads) supports live
+debugging.  The counters are exposed programmatically as
+`SPCommunicator.coherence_counters`.
+
+The two rejection buckets split on whether *this* rank's own sources
+disagreed, not on the field's policy: a relaxed field can straddle a
+publish too (its floor then differs from a peer reader's and the
+collective check rejects), and that is this rank's coherence miss.
+`rejected_cross_reader` is the shadow such a miss casts on the other
+reader ranks, so the summary's `miss rate`
+(`coherence_miss_rate`) counts `rejected_incoherent` **and**
+`rejected_cross_reader` and `accepted_mixed` — equivalently, every read
+that was neither a clean accept nor a clean nothing-to-take.  Counting
+only the locally-detected misses would divide the rate by the number of
+reader ranks, since one straddle on an R-rank reader records one
+`rejected_incoherent` and R-1 `rejected_cross_reader`.
+
 
 ### Impact on Existing Components
 
@@ -700,9 +738,9 @@ differs from 1.0)
 - Add the `fullcomm.allgather` layout exchange for the unequal-rank
   path (Option D's addressing), *alongside* the existing
   `strata_comm`-based exchange, which the equal-rank path keeps using.
-  This is the interim exchange; the scalable replacement is a release
-  gate, not a feature phase (see the Option D layout-exchange note and
-  §Gate reliance on the feature with an MPI CI matrix).
+  (This flat allgather is the permanent exchange, not an interim one —
+  see the Option D layout-exchange note for why a "scalable"
+  replacement was rejected.)
 - Implement multi-source `get_receive_buffer()` using overlap maps, as
   a path taken only under non-default ratios; the single-source reader
   is unchanged for the equal-rank case.
@@ -905,13 +943,13 @@ two MPI implementations (e.g. OpenMPI and MPICH) and more than one
 mpi4py / MPI version, since that path is where the RMA-portability risk
 lives.
 
-The same "finish before recommending it" list carries the **scalable
-layout exchange**: the interim `fullcomm.allgather` (see the Option D
-layout-exchange note) must be replaced by the two-level or local-compute
-scheme before the feature is documented or recommended for production
-use, because total rank counts in the thousands are a real operating
-regime here.  Both are prerequisites for recommending the feature, not
-for landing the intervening phases on `main`.
+A **scalable layout exchange** used to sit on this same "finish
+before recommending it" list; it was removed after the scaling
+analysis showed the flat `fullcomm.allgather` is fine at any realistic
+rank count (Pyomo/mpi-sppy#726, closed won't-fix; see the Option D
+layout-exchange note) — leaving the MPI-implementation matrix above as
+the remaining prerequisite for recommending the feature (not for
+landing the intervening phases on `main`).
 
 
 ### Possible future work (out of scope)
@@ -926,17 +964,6 @@ scenario, saving storage and bandwidth on those fields.  This is *only*
 valid for the Category-2 fields — never for the Category-1 per-scenario
 fields — and is explicitly **not** required for flexible ranks.
 
-**Coherence read-outcome diagnostic** (Pyomo/mpi-sppy#742).  An
-always-on, per-field counter at the multi-source reader breaking each
-read into `not_new` / `new_accepted` / `rejected_incoherent` /
-`accepted_mixed`.  This lets an infrequently-reporting bounds cylinder be
-diagnosed as a coherence problem (reads rejected because sources disagree
-on `write_id`) vs. a slow upstream sender (no new data), and measures how
-often a multi-source read straddles a publish — the empirical basis for
-the strict-vs-relaxed choices above, especially under an asynchronous APH
-sender.
-
-
 ### Backward Compatibility
 
 When all rank ratios are 1.0 (the default), the system behaves

mpisppy/cylinders/spcommunicator.py

@@ -111,6 +111,28 @@ def reduce_source_write_ids(source_ids, strict: bool) -> int:
         return source_ids[0] if len(set(source_ids)) == 1 else -1
     return min(source_ids)
 
+
+def coherence_miss_rate(counters) -> float:
+    """Fraction of the multi-source reads in ``counters`` that a straddled
+    publish cost something (see SPCommunicator._count_coherence_read).
+
+    All three non-clean outcomes count, not just the locally-detected one: a
+    read this rank rejected because its own sources disagreed
+    (``rejected_incoherent``), one rejected because a *peer* reader rank
+    straddled and broke cross-reader agreement (``rejected_cross_reader``), and
+    one accepted with a blended assembly (``accepted_mixed``). Counting only
+    the first would divide by the number of reader ranks: one straddle on an
+    R-rank reader records 1 ``rejected_incoherent`` and R-1
+    ``rejected_cross_reader``.
+    """
+    if counters["total"] == 0:
+        return 0.0
+    misses = (counters["rejected_incoherent"]
+              + counters["rejected_cross_reader"]
+              + counters["accepted_mixed"])
+    return misses / counters["total"]
+
+
 def communicator_array(data_length: int):
     """
     Allocate an MPI memory region with a padded length (multiple of 8 doubles = 64B),
@@ -347,6 +369,25 @@ def __init__(self, spbase_object, fullcomm, strata_comm, cylinder_comm, communic
         self.overlap_maps = {}            # -> list[OverlapSegment] (global ranks)
         self._overlap_source_ranks = {}   # -> sorted distinct source global ranks
 
+        # Per-field read-outcome counters for the unequal-rank multi-source
+        # reader (see _count_coherence_read); always accumulated (two integer
+        # increments per multi-source read), reported at finalization by
+        # report_coherence_diagnostics. Empty on the equal-rank path and for
+        # single-source reads, which cannot straddle a publish.
+        self.coherence_counters = {}
+        # opt-in periodic per-field line for live debugging: print local
+        # counters every N multi-source reads (0 = off). Read from the
+        # underlying SPBase options, not this object's `options`: the bound
+        # spoke constructors take no `communicators` argument, so the cylinder
+        # list WheelSpinner passes positionally lands in their `options`
+        # parameter and `SPCommunicator.options` is always empty on a spoke.
+        # opt.options is set from opt_kwargs for every cylinder, and is where
+        # the sibling cylinder-wide debug switches (`trace_prefix`,
+        # `inspect_buffers_on_shutdown`) already live.
+        self._coherence_report_period = int(
+            self.opt.options.get("coherence_diagnostics_period", 0)
+        )
+
         # setup FieldLengths which calculates
         # the length of each buffer type based
         # on the problem data
@@ -893,15 +934,33 @@ def _flex_get_multi_source(self, buf, field, peer_cylinder, synchronize):
             source_snapshots[r] = snapshot
             source_ids.append(int(snapshot[logical_len - 1]))
 
-        new_id = reduce_source_write_ids(
-            source_ids, strict=field in _STRICT_COHERENCE_FIELDS
-        )
+        strict = field in _STRICT_COHERENCE_FIELDS
+        new_id = reduce_source_write_ids(source_ids, strict=strict)
+
+        # Read-outcome diagnostic: count genuinely multi-source reads (>= 2
+        # sources; a single source cannot straddle a publish). The outcome
+        # buckets let a user tell a coherence problem (reads rejected or
+        # blended) from a slow upstream sender (nothing new to read) when a
+        # consumer appears to report infrequently.
+        counters = None
+        if len(source_ids) >= 2:
+            counters = self._count_coherence_read(field)
+            mixed = len(set(source_ids)) > 1
 
         if not self._write_ids_agree(new_id, synchronize):
+            if counters is not None:
+                # this rank's own sources disagreeing is the fundamental
+                # coherence miss (the read straddled a publish) whatever the
+                # field's policy; otherwise its sources agreed and it was the
+                # collective cross-reader check that rejected the read
+                counters["rejected_incoherent" if mixed
+                         else "rejected_cross_reader"] += 1
             buf._is_new = False
             return False
 
         if new_id > last_id:
+            if counters is not None:
+                counters["accepted_mixed" if mixed else "new_accepted"] += 1
             # assemble the accepted data into buf, then commit via the shared
             # _mark_new (which stamps the id slot the assembly does not touch)
             data_view = buf.value_array()
@@ -910,9 +969,92 @@ def _flex_get_multi_source(self, buf, field, peer_cylinder, synchronize):
                 data_view[seg.local_offset : seg.local_offset + seg.count] = \
                     snapshot[seg.remote_offset : seg.remote_offset + seg.count]
             return self._mark_new(buf, new_id)
+        if counters is not None:
+            # strict + mixed lands here when every reader rank computed the
+            # sentinel -1, so cross-reader agreement held but the id cannot
+            # advance -- still a coherence rejection, not a slow sender. A
+            # *relaxed* mixed read that gets here is the slow-sender case
+            # proper: the floor did not move because one source is behind.
+            counters["rejected_incoherent" if strict and mixed
+                     else "not_new"] += 1
         buf._is_new = False
         return False
 
+    def _count_coherence_read(self, field: Field) -> dict:
+        """Count one multi-source read of ``field`` and return its outcome
+        counters (created on first use) for the caller to bucket:
+
+          * ``new_accepted`` -- sources agreed on an advanced write_id; used.
+          * ``not_new`` -- the write_id did not advance, so there was nothing
+            to take (the sender has not published since the last accepted
+            read). A relaxed field whose sources disagree but whose floor has
+            not moved lands here too: some source has not published yet, which
+            is the same diagnosis.
+          * ``rejected_incoherent`` -- this rank's sources disagreed and the
+            read was rejected, so it will be retried (the fundamental coherence
+            miss: the read straddled a publish).
+          * ``rejected_cross_reader`` -- this rank's sources agreed, but the
+            collective cross-reader write_id check rejected the read (some
+            other rank of this cylinder saw a different id -- typically because
+            *it* straddled the publish, and records the miss itself).
+          * ``accepted_mixed`` -- a relaxed field's sources disagreed and the
+            blended assembly was used anyway.
+
+        The buckets partition ``total``. The coherence miss rate is
+        ``coherence_miss_rate(counters)`` -- every read that a straddled
+        publish cost something, whether it was rejected here, rejected because
+        a peer reader straddled, or accepted blended. If ``not_new`` dominates
+        instead, the upstream sender is just slow.
+        """
+        counters = self.coherence_counters.setdefault(field, {
+            "total": 0,
+            "new_accepted": 0,
+            "not_new": 0,
+            "rejected_incoherent": 0,
+            "rejected_cross_reader": 0,
+            "accepted_mixed": 0,
+        })
+        counters["total"] += 1
+        if self._coherence_report_period > 0 and self.cylinder_rank == 0 \
+                and counters["total"] % self._coherence_report_period == 0:
+            # live-debugging line: this rank's counts only (the current
+            # read's outcome bucket is not yet incremented)
+            print(f"coherence diagnostic [{self.__class__.__name__}] "
+                  f"{field.name}: "
+                  + ", ".join(f"{k}={v}" for k, v in counters.items()),
+                  flush=True)
+        return counters
+
+    def report_coherence_diagnostics(self):
+        """Print a per-field summary of the multi-source read outcomes
+        accumulated in ``coherence_counters`` (see _count_coherence_read for
+        the buckets and their diagnosis). Collective on ``cylinder_comm``:
+        every rank of the cylinder must call it (different ranks can have
+        different multi-source fields, or none, so the counters are gathered
+        rather than reduced); rank 0 prints. Inert -- no output, one gather --
+        at equal ranks or when no multi-source reads happened.
+        """
+        if not self._flex_ranks:
+            return
+        all_counters = self.cylinder_comm.gather(self.coherence_counters, root=0)
+        if self.cylinder_rank != 0:
+            return
+        totals = {}
+        for rank_counters in all_counters:
+            for field, counters in rank_counters.items():
+                aggregate = totals.setdefault(field, dict.fromkeys(counters, 0))
+                for outcome, count in counters.items():
+                    aggregate[outcome] += count
+        for field in sorted(totals):
+            counters = totals[field]
+            if counters["total"] == 0:
+                continue
+            print(f"coherence diagnostic [{self.__class__.__name__}] "
+                  f"{field.name}: "
+                  + ", ".join(f"{k}={v}" for k, v in counters.items())
+                  + f", miss rate={coherence_miss_rate(counters):.2%}",
+                  flush=True)
+
     def receive_nonant_bounds(self):
         """ receive the bounds on the nonanticipative variables based on
         Field.NONANT_LOWER_BOUNDS and Field.NONANT_UPPER_BOUNDS. Updates the

mpisppy/spin_the_wheel.py

@@ -179,6 +179,10 @@ def run(self, comm_world=None):
         # Anything that's left to do
         spcomm.finalize()
 
+        # Unequal-rank runs: per-field multi-source read-outcome summary
+        # (collective on cylinder_comm; inert at equal ranks)
+        spcomm.report_coherence_diagnostics()
+
         # to ensure the messages below are True
         cylinder_comm.Barrier()
         global_toc(f"Hub algorithm {opt_class.__name__} complete, waiting for spoke finalization", comm_world.rank == 0)