test(analysis): measure the numeric boundaries instead of assuming them - #52
Merged
Merged
Conversation
14 tasks
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
Second numeric-integrity step for #1, whose criteria ask for an audit of the CSV, JSON, database, browser, R and external-tool boundaries for precision loss, integer range and serialization compatibility. An audit that only restates the contract is not an audit, so each boundary here is crossed with a real partner: node for the browser, Rscript for R, DuckDB for the database. Measured, with the probe being 9007199254740993 (2^53+1), the smallest integer Float64 cannot hold: - Browser. In a real JavaScript engine, that count as a JSON NUMBER arrives as 9007199254740992 -- one short, unmarked -- while the encoded STRING crosses byte for byte. This is the whole reason encode_json_number exists, now demonstrated rather than asserted. - R. R has no integer type wider than 32 bits (as.integer(2^31) is NA), every number is a double, and neither format() nor as.character() can render that count back to its digits. A count that has been through R is therefore unproven, and the contract's diagnosis -- send and read it as a string -- is asserted by round trip through Rscript, since what R never converts, R cannot corrupt. - Database. A BIGINT column carries the count exactly and a DOUBLE column silently changes it, so the column type is part of the contract. Past BIGINT a BIGINT column refuses (2^64 raises) rather than rounding, HUGEINT is exact to 2^127, and beyond that the string form transports the value unchanged. - CSV. A CSV file is text, so the boundary is the parser's; asserted through the round-trip testset, where "9.007199254740992e15" and "9007199254740993.0" are refused by name. Closes the round trip as an executable contract: parse_exact_count and parse_exact_rational are the inverses of to_storage, accepting a whole-number literal and nothing else. A value that has passed through a float cannot be inspected for the damage, so the string form is the only one allowed to claim to be a count -- and the negative control asserts that the float form of that literal really is a different number. Skips, where they happen, are loud and named ("browser (node absent -- the JavaScript boundary is untested here)") rather than a quiet pass, and the JavaScript is written with destructuring because the repository's own lint_source.jl reads a qualified `JSON.parse` inside test prose as a Julia module reference -- writing the JS idiomatically is cheaper than teaching the gate an exception. Tests: 45 boundary assertions alongside the existing 144, both run under the real module. Refs #1
hyperpolymath
force-pushed
the
feat/numeric-boundaries
branch
from
September 22, 2026 10:03
6af6cb6 to
79e2333
Compare
|
❌ Failed to create Coding Agent finishing-touch task. Please try again. |
|
|
❌ Failed to create Coding Agent finishing-touch task. Please try again. |
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.
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 — second numeric-integrity step, following #49. This one closes the issue's
boundary-audit criterion: "audit CSV, JSON, database, browser, R and external-tool
boundaries for precision loss, integer range and serialization compatibility."
An audit that restates the contract is not an audit, so every boundary is crossed
with a real partner — node for the browser, Rscript for R, DuckDB for the database.
The probe throughout is 9007199254740993 (
2^53+1), the smallest integer Float64cannot hold.
9007199254740992— one short, unmarked; the encoded string crosses byte for byteas.integer(2^31)isNA), every number is a double, and neitherformat()noras.character()renders that count back to its digitsBIGINTcarries the count exactly,DOUBLEsilently changes it; pastBIGINTaBIGINTcolumn refuses (2^64raises) instead of rounding;HUGEINTis exact to2^127; beyond that the string form transports it unchanged"9.007199254740992e15"and"9007199254740993.0"are refused by nameThe round trip is now executable
parse_exact_countandparse_exact_rationalare the inverses ofto_storage: awhole-number literal, and nothing else. This matters because a value that has passed
through a float cannot be inspected for the damage — so the string form is the only
representation allowed to claim to be a count. The negative controls assert that the
float form of that literal really is a different number, so the refusals are not
ceremony.
Skips are loud
Where a partner is absent the testset is named —
browser (node absent — the JavaScript boundary is untested here)— so a missing boundary check shows in thesummary rather than hiding inside a clean run. #30 is what a silently skipped check
costs.
One note on the repository's own lint
config/ci/lint_source.jlscans forCapitalised.nameas a Julia module reference, andread a qualified
JSON.parseinside my JavaScript text as a module namedJSONthefile never imports. I wrote the JS with destructuring (
const { parse } = JSON) ratherthan teaching the gate an exception for prose it cannot see into — the gate is right to
be suspicious of that pattern, and the fix costs one line of idiomatic JavaScript.
Verification
45 boundary assertions alongside the existing 144 numeric-policy tests, both run
against the real module here.
check-format,check-spdxandlint_source.jlclean.Not claimed: anything about methods — the method catalogue still awaits your approval,
per the issue's own ordering.
Branch rebuilt. When I opened this I had branched from the still-open gate branch, so
this PR briefly carried the gate fix as well. That fix has since merged as #51
(
0ef5eee ci(tools): retry pinned downloads and name a TLS failure as one), so I rebuiltthis branch on current
mainand force-pushed with a lease. This PR now carries onecommit and three files, all numeric:
src/analysis/numeric_policy.jl,test/unit/test_numeric_boundaries.jl,test/runtests.jl.