Skip to content

Preserve federated quantiles across empty client digests - #5364

Open
sylvesterkaczmarek wants to merge 1 commit into
NVIDIA:mainfrom
sylvesterkaczmarek:fix/preserve-quantiles-across-empty-clients
Open

sylvesterkaczmarek wants to merge 1 commit into
NVIDIA:mainfrom
sylvesterkaczmarek:fix/preserve-quantiles-across-empty-clients

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Fixes #5363.

Preserve the accumulated t-digest when a client supplies no digest, and replace an empty placeholder when a later client has data. This prevents client-order-dependent loss of quantiles and AttributeError from calling .merge() on an empty dictionary. All-empty features continue returning unavailable quantiles.

The regression exercises the public get_quantiles path using real fastdigest objects. It covers all six orders of two populated clients and one empty client, checks expected quantiles and unchanged serialized inputs, and retains an all-empty control.

Validation

python -m pytest -q tests/unit_test/app_common/statistics

92 passed on Linux ARM64, Python 3.12, with the project's pinned fastdigest==0.4.0. The focused regression on unchanged upstream gives 6 failures and 1 pass. Black, isort, flake8 and git diff --check pass for the changed files.

This was tested against the checkout's source in an isolated Linux container. A deployed federated job was not run.

Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Fixes quantile merging logic for empty client digests.

The PR appears safe to merge; no actionable issue was identified.

Summary

The PR preserves accumulated quantiles across empty client digests.

  • Replaces an empty placeholder when a later client contributes a digest.
  • Adds order-permutation and all-empty regression tests through get_quantiles.

Reviews (1) · Last reviewed commit: "Preserve federated quantiles across empt..."

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty client digests overwrite accumulated federated quantiles

1 participant