Skip to content

fix(metrics): weight aggregation by samples, and report round ESS - #102

Open
ajbarea wants to merge 4 commits into
mainfrom
aj-num-examples-and-ess
Open

ajbarea wants to merge 4 commits into
mainfrom
aj-num-examples-and-ess

Conversation

@ajbarea

@ajbarea ajbarea commented Sep 24, 2026

Copy link
Copy Markdown
Owner

num-examples was set from len(trainloader) / len(testloader) at all four call sites in client_app.py. len() on a DataLoader counts batches, not examples.

Confirmed against torch at batch_size=32:

samples len(dataloader) len(dataloader.dataset)
33 2 33
64 2 64
500 16 500
501 16 501

flwr 1.38's FedAvg takes weighted_by_key="num-examples" by default, so that batch count was weighting both the LoRA adapter aggregate and the reported loss/accuracy. Since batches are ceil(n/32), the error is a quantisation that systematically over-weights the smallest partitions — largest exactly under the Dirichlet skew the testbed exists to study. Now len(loader.dataset).

fl.round.ess

Effective sample size, 1/Σwᵢ² over the same num-examples weights FedAvg aggregates by. Equals the client count when shares are even, falls toward 1.0 as one client dominates, NaN when nothing aggregated. Emitted as a round metric and an fl.ess span attribute.

Participation counts report how many clients replied, not how much each one moved the aggregate. On the four partition sizes above, ESS reads 2.38 over sample counts against 4 clients — this metric is what makes that class of weighting bug visible instead of silent.

Verification

ruff format --check and ruff check pass locally. ty check and pytest need the app env, and ray ships no macOS x86_64 wheel, so uv sync cannot build on an Intel Mac — those ride on CI here.

Follow-up, not in this PR

Each client evaluates on a 20% holdout carved from its own partition, so the round figure is a weighted mean over local shards rather than a global test set. Under Dirichlet the holdout inherits the partition's label skew. Filed as a ROADMAP v2 item, since fixing it changes the experiment rather than the plumbing.

`num-examples` was set from `len(trainloader)` / `len(testloader)` at all four
call sites. `len()` on a DataLoader counts batches, so 33 samples and 64 samples
both reported 2 at batch_size=32.

flwr's FedAvg defaults to `weighted_by_key="num-examples"`, so that batch count
weighted the adapter aggregate and the reported loss/accuracy. The `ceil(n/32)`
quantisation over-weights the smallest partitions, which is worst under the
Dirichlet skew this testbed studies. Use `len(loader.dataset)`.

Add `fl.round.ess`: effective sample size, `1/Σwᵢ²` over those same weights.
Equal to the client count on even shares, falling toward 1.0 as one client
dominates, NaN when nothing aggregated. Participation counts cannot show
concentration; this can, and would have made the batch-count weighting visible.
@ajbarea ajbarea added the bug Something isn't working label Sep 24, 2026
ty rejects float() on a MetricRecord value: the union includes Array. Same
situation _round_summary already handles, so use the same idiom.
Four defects from review, each with a regression test.

_num_examples indexed msg.content["metrics"] by literal record name. flwr
addresses the record by type, so a reply naming its MetricRecord anything else
raised KeyError inside aggregate_train — a telemetry read aborting the round it
only meant to observe. Read metric_records by type; return None, never raise.

ESS as 1/Σ(wᵢ/Σw)² is not float-exact: an even five-way split read
4.999999999999999, ten-way 9.999999999999996. Kish's (Σwᵢ)²/Σwᵢ² over the raw
weights with math.fsum is exact for every n in 2..32.

The four changed client_app lines ran in no test: nothing imports the module,
the CI job named smoke-test only runs `flwr build`, and ty sees torch.** as Any.
Extract _sample_count and cover it.

Add fl.ess to the docs' round-attribute and metric enumerations, which were
also already missing fl.failures.
…have

flwr.supercore.task_identity arrives in 1.38; the lock pins 1.36, where
Message constructs with no process identity. Verified the test_server
assertions against 1.36.0.
@ajbarea

ajbarea commented Sep 24, 2026

Copy link
Copy Markdown
Owner Author

Review notes

Four defects in the first cut of this branch, each now fixed and covered by a test.

1. server_app.py — a telemetry read could abort the round. _num_examples indexed msg.content["metrics"] by literal record name, but flwr addresses the MetricRecord by type throughout its own aggregation. Built a reply on flwr 1.36 whose record is named anything else and confirmed msg.content["metrics"] raises KeyError — inside aggregate_train, before super() runs, so an observability read would kill the aggregation it only meant to observe. Now reads metric_records by type and returns None instead of raising. Fixed in e5540b0.

2. server_app.py — ESS was not float-exact. 1/Σ(wᵢ/Σw)² returns 4.999999999999999 for an even five-way split and 9.999999999999996 for ten; the original tests only asserted n=2 and n=4, which happen to land exact. Kish's (Σwᵢ)²/Σwᵢ² over the raw weights with math.fsum is exact for every n in 2..32, now asserted across that range. Clamping with min(ess, n) was considered and rejected — the error runs below n, not above. Fixed in e5540b0.

3. client_app.py — the four changed lines ran in nothing. No test imported the module, the CI job named smoke-test runs only flwr build, and ty resolves torch.** to Any, so a runtime error on len(loader.dataset) would have failed every client on every round with all twelve checks green. Extracted _sample_count and added tests/test_client.py, covering the row/batch collisions (33 vs 64, 500 vs 501), batch-size independence, and an empty partition. Fixed in e5540b0.

4. Docs omitted the new metric. docs/architecture.md and docs/getting-started.md both enumerate the round span attributes and metric names; neither listed fl.ess, and neither listed fl.failures either, which predates this branch. Both now complete. Fixed in e5540b0.

Refuted

Suspected that run manifests written before this change recorded the batch-count convention under the same num-examples field, making old and new runs quietly incomparable. They do not: flwr's aggregate_metricrecords excludes the weighting key from the aggregated record, so num-examples never reaches evaluate_metrics_clientapp or the manifest.

Verified clean

  • Read flwr 1.36's FedAvg: weighted_by_key="num-examples" is the default, and aggregate_arrayrecords weights the LoRA tensors by it — so the batch count was skewing the adapter aggregate, not only the reported metrics.
  • len(DataLoader) vs len(DataLoader.dataset) reproduced against torch: 33 and 64 rows both report 2 batches at batch_size=32; 500 and 501 both report 16.
  • Traced every num-examples / num_examples site and every caller of record_round_metrics, record_client_metrics and effective_sample_size across the tree. record_round_metrics gained ess with a NaN default, so the existing call in tests/test_telemetry.py is unaffected.
  • ESS assertions re-run against the locked flwr 1.36.0 specifically, after an earlier helper was written against 1.38 and failed here.
  • ESS scale-invariance, the skew case, and both degenerate inputs (empty, all-zero) return NaN rather than raising.
  • 29 tests collected and passing on 3.12 and 3.13, tests/test_client.py among them.

Not exercised end to end: no live flwr run has gone through the client_app change — tests/test_client.py covers the expression, not a federated round.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant