Repository navigation
docs(statistics): publish the conditions for exact descriptive summaries - #56
Merged
Merged
Conversation
The approved catalogue requires every method to publish its supported and
unsupported conditions BEFORE it is implemented, so that the implementation
can be held to terms the reviewer saw first. This is that document for
item 1, the first method in the approved v1 scope, and it exists ahead of
any code deliberately.
It commits the method to the things that are easy to leave implicit:
- Counts and proportions are exact -- and a proportion for a zero-total
sample is UNDEFINED, never NaN, never 0/0 and never 0. Reporting zero
there is a lie with a straight face, and this is the sort of default that
is only ever fixed before the code exists.
- A text fraction ("2/3") is exact; a float is an approximation and is
labelled as one wherever it is shown. Nothing is presented as exact
because it happens to look exact.
- Per-group summaries are NOT a comparison, and neither the document nor
the user-facing text lets that be misread: no test is performed, so the
mandatory-BH rule does not apply, and saying so turns an omission into a
recorded decision.
- Depth always travels with proportions; a proportion without its
denominator is uninterpretable for compositional data.
- Budget overruns raise rather than round. A silent downgrade from exact to
approximate is the failure this layer exists to prevent.
It also fixes the evidence a reviewer should demand when the implementation
arrives: hand-derived known answers, an independent reference compared value
by value (skipping loudly by name if the runner lacks the partner), negative
controls for the zero-total case and for a float claiming exactness, and
proof that behaviour is unchanged at the :ordinary default.
Documentation only; no code changes. check-format and check-spdx clean.
Refs #1
|
Warning Review limit reachedNext included review available in 42 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
hyperpolymath
added a commit
that referenced
this pull request
Sep 22, 2026
Refs #1 — **the first method under the approved catalogue**, and the first held to conditions that were published while it did not yet exist (`docs/statistics/method-conditions/exact-descriptive-summaries.md`, merged in #56). Counts and proportions per sample and per group, carried exactly. No inference is claimed, and the module has no vocabulary for one — no p-value, no interval, no model, no comparison. ## The distinction the code exists to keep A single number cannot tell two different facts apart: | | | | --- | --- | | A feature with **no reads**, in a sample that has reads | relative abundance of **exactly 0** — a value | | A sample with **no reads at all** | **no** relative abundances — carried as `nothing`, never `0` | That second case is the one `Execution.jl` writes `0.0` for today. The exact layer reports it as undefined; the legacy path is deliberately **left alone**, because changing it would alter results that saved analyses were computed from. ## Decisions worth naming - **Float input is accepted only when finite, integral and within 2^53**, and the result is marked `approximate` and says so in its warnings. Beyond 2^53 a float cannot say which integer it holds — the boundary audit (#52) showed exactly what that costs. - **An ordinary or high-precision policy is refused by name.** Higher precision is not exactness, and a caller who asked for exactness must not receive Float64s that look identical until somebody checks. - **Group summaries aggregate counts, not proportions.** A mean of proportions weights a shallow sample like a deep one, which is a different (and usually unintended) statement. - **Storage and display stay separate.** Stored proportions are strings that parse back through `parse_exact_rational` to the identical rational; a rounded decimal is never what gets written down. ## Making the code match the conditions fixed two things underneath Rather than adjust the document to fit the code, the code changed — which surfaced two display defects in the numeric layer: 1. `to_display` printed a rendering labelled **"6dp"** followed by ~80 digits; `round(x; digits)` on a BigFloat keeps its 256-bit significand, so the label was right and the rendering was not. 2. It rendered exact rationals as Julia's **`2//3`** — syntax leaking into text a person reads, and not the form they could copy out. Display now agrees with `to_storage`. The rounding is now done in **integer arithmetic** (scale, `divrem`, half away from zero) rather than by formatting a BigFloat. That keeps a value which is exactly not a float away from float arithmetic and from whatever rounding rule the float libraries use, it renders correctly past Float64 (a rational with a 2^200-scale integer part prints its digits, with no exponent notation in text meant to be read), and it needs **nothing beyond Base** — after a first attempt added a stdlib dependency the package did not declare and could not precompile. ## Evidence (all four the conditions require) | Required | Delivered | | --- | --- | | Hand-derived known answers | totals, `2/3`, `1/3`, `1//5 + 2//5 + 2//5 == 1` exactly, `2^53+1` as a count | | Independent reference | compared value by value against Python `fractions.Fraction`; skips **loudly by name** if `python3` is absent | | Negative controls | zero-total → undefined (not 0, not NaN), float claiming exactness refused, negative count refused, budget overrun raises | | Unchanged when unselected | a guard fails **by name** if the pipeline starts calling this module | Plus display tests for the cases a float-based renderer gets wrong: signs (`-2/3` must not print `--`), true ties (`1/8` at 2dp rounds away from zero in *both* directions), and values beyond Float64. **Mutation-tested**, because a test that cannot fail is decoration: making a zero-total sample report `0` fails four assertions; wiring the pipeline to this module fails the guard. Removing the `:widen` flag does **not** fail them — counts are already `BigInt` by then, so the flag is defensive rather than load-bearing — and the test says so out loud rather than implying a mechanism test it does not perform. ## Also in this branch: the gate that let the dependency bug through The first push failed CI precompilation with `Package MetaManifold does not have Printf in its dependencies` — a bug this repo already has a lint check for. That check had two false negatives and the bug walked through both: it exempted a list of "stdlibs needing no `[deps]` entry" (Printf among them — there is no such class of name; only `Base`, `Core` and `Main` are bound without a declaration), and it examined only the first name in a comma list, so `using JSON3, YAML` never checked YAML. Rewritten to read the AST rather than pattern-match, with a **self-test that replays the input which defeated it** — an undeclared stdlib, and a name after the first in a comma list — alongside the inverses that must *not* be reported. Mutation-checked: adding Printf to the always-bound set makes the self-test fail and name the reason. **Tests**: 97 new assertions; the numeric policy (144) and boundary (45) suites still pass. `check-format`, `check-spdx`, `lint_source.jl` clean. Nothing is wired into the UI yet — that is a separate step and a separate decision.
14 tasks
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
1 participant
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.
Refs #1 — the catalogue's own precondition, for the first method it unblocks.
The approved
method-catalogue-v1.mdrequires every method to publish its supported andunsupported conditions before it is implemented, so a reviewer can hold the code to
terms they saw first. This is that document for item 1 — exact descriptive summaries,
and it deliberately exists ahead of any code.
What it commits the method to
undefined, never
NaN, never0/0, never0. That is the kind of default that onlyever gets fixed before the code exists.
"2/3") is exact; a float is an approximation and is labelled asone wherever it is shown. Nothing is presented as exact because it merely looks exact.
mandatory-BH rule does not apply — and saying so turns an omission into a recorded
decision rather than something a reader has to infer.
uninterpretable for compositional data.
approximate is precisely the failure this layer exists to prevent.
:ordinarydefault.What a reviewer should demand with the implementation
runner lacks the partner, so "no independent reference ran" is visible.
denominator-budget overrun.
Documentation only;
check-formatandcheck-spdxclean. The implementation PR followsthis one, and will be held to it.