Skip to content

orb(federated): the peer "median" uses nearest-rank percentile #9645

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

src/orb/analytics.ts ships two different implementations of "the median" and they disagree on even-sized
inputs:

function median(xs: number[]): number | null {           // line 167
  ...
  return s.length % 2 === 0 ? (s[mid - 1]! + s[mid]!) / 2 : s[mid]!;
}

export function percentile(sorted: number[], p: number): number | null {   // line 174
  const idx = Math.min(sorted.length - 1, Math.max(0, Math.ceil((p / 100) * sorted.length) - 1));
  return sorted[idx]!;
}

For [0.4, 0.8], median returns 0.6; percentile(_, 50) returns 0.4.

refreshFederatedBenchmarkCache (src/orb/federated-benchmark.ts:148-157) computes the peer half of the
#6481 dashboard comparison with percentile(peerMergePrecisions, 50) under a comment that says the opposite of
what the code does:

MEDIAN, NOT MEAN (mirrors analytics.ts's own fleet aggregation, see federated-import.ts's header comment):
a bounded number of outliers cannot drag a median arbitrarily, so re-deriving a mean here would quietly
weaken the same poisoning-resistance property the import side already relies on holding by construction.

It does not mirror analytics.ts's fleet aggregation: computeFleetAnalytics uses median()
(src/orb/analytics.ts:325-326, 388-393) for fleet.mergePrecision, the very quantity a peer bundle publishes
and this cache aggregates. So the fleet-side median of a set of instance precisions and the peer-side "median"
of the same kind of numbers are computed by two different estimators, and the comment asserting they are the
same is actively misleading to anyone reasoning about the poisoning-resistance argument it makes. (Ironically
analytics.ts's own median IS a mean at even n — the exact thing this comment says must not happen.)

percentile also carries an undocumented precondition: its parameter is named sorted and it does not sort.
refreshFederatedBenchmarkCache sorts before calling (line 151) and computeFleetAnalytics relies on a SQL
ORDER BY (src/orb/analytics.ts:274-278), but nothing enforces it, and median — the sibling with the same
apparent purpose — sorts internally. Two functions with opposite input contracts and no doc stating either is a
standing trap for the next caller.

Requirements

  • refreshFederatedBenchmarkCache must use the SAME estimator as computeFleetAnalytics's fleet median.
    Export median from src/orb/analytics.ts and call it, deleting the local sort at
    src/orb/federated-benchmark.ts:151 (median sorts internally).
  • The comment at src/orb/federated-benchmark.ts:143-147 must be corrected so it describes what the code does:
    it now genuinely uses analytics.ts's median, and the sentence claiming a mean would weaken the property
    must be reworded to state that median averages the two middle values at even n by design.
  • percentile's doc comment (src/orb/analytics.ts:174) must state its precondition explicitly: the input
    MUST already be ascending-sorted, and the function does not sort. Name the two call sites that satisfy it.
  • median's doc must state that it sorts a copy internally and returns the mean of the two middle values at
    even n, so the two functions' contracts are legible side by side.
  • cycleP50Ms/cycleP95Ms (src/orb/analytics.ts:394-395) must keep using percentile — they are
    percentiles, not medians, and their values must not change.

⚠️ Required pattern: median in src/orb/analytics.ts:167-172 is the single fleet-aggregation estimator;
the federated peer half must call it, not a second implementation. What does NOT satisfy this issue: changing
percentile's p=50 behaviour (that would silently move cycleP50Ms); adding a third "median" helper in
federated-benchmark.ts; or fixing only the comment while leaving the two estimators divergent.

Deliverables

  • median is exported from src/orb/analytics.ts and refreshFederatedBenchmarkCache uses it for
    peerMedianMergePrecision, with the local .sort(...) removed.
  • A named regression test asserting that for two accepted peer bundles with mergePrecision 0.4 and
    0.8, peerMedianMergePrecision is 0.6 — where today it is 0.4.
  • A test asserting cycleP50Ms and cycleP95Ms are unchanged for an existing fixture (the percentile
    path must not move).
  • The comment block at src/orb/federated-benchmark.ts:143-147 describes the estimator actually used.
  • percentile's doc states the pre-sorted precondition; median's doc states the internal sort and the
    even-n averaging.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example fixing
the comment without switching the estimator — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. Both src/orb/analytics.ts and
src/orb/federated-benchmark.ts are inside src/**, which is inside coverage.include, so both are measured
and gated. median's odd-length and even-length arms both need a test through the new call site, as does the
empty-array arm (zero accepted peers ⇒ null). The regression test in Deliverable 2 is mandatory.

Expected Outcome

The local half and the peer half of the #6481 "your gate precision vs peer median" comparison are aggregated by
one estimator, and every comment in the federated pipeline describing that estimator is true of the code it sits
above.

Links & Resources

src/orb/federated-benchmark.ts:143-158, src/orb/analytics.ts:167-182, 320-326, 384-395,
src/orb/federated-bundle.ts:31, src/orb/federated-import.ts (the poisoning-resistance argument the comment
cites).

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions