fix(igv): bound the JSON features any one track may emit (closes #129) - #132
Merged
Conversation
Closes #129. `rs12740374_SORT1_legnet_report.html` regenerated at 131 MB (from 1.29 MB) and the consolidated multi-oracle report at 139 MB — both above GitHub's hard 100 MiB per-file limit, so neither could be committed in any form. That blocked regenerating the multi-oracle example at all. Root cause is a missing cap, not tiling. `_calculate_track_bin_size` returned the bare `resolution` for legnet, which makes `bins_per = max(1, bin_size // resolution)` equal 1 inside `_downsample_to_features` — one JSON feature per input bin, no downsampling whatever. Every other oracle fell through to a 3,000-feature budget. Measured features per track before this change: legnet res=1 window=200,000 -> 200,000 OVER legnet res=1 window=1,048,576 -> 1,048,576 OVER chrombpnet res=1 window=200,000 -> 10,000 OVER sei res=1 window=4,096 -> 4,096 OVER chrombpnet res=1 window=2,114 -> 106 alphagenome res=1 window=1,048,576 -> 3,005 enformer res=128 window=114,688 -> 896 borzoi res=32 window=524,288 -> 3,277 So three oracles were unbounded, not just legnet — legnet was simply the one that got wide enough to break the 100 MiB limit. The fix keeps each oracle's *preferred* bin size and aggregation, then applies `_MAX_FEATURES_PER_TRACK = 4_000` as a floor on the bin width for everyone. Two properties matter: - It only binds on wide windows. At ChromBPNet's real 2,114 bp input the budget floor is 1 bp, so its deliberate 20 bp max-pooling (PR #79: mean-pooling a 1 bp profile dilutes sharp peaks below the p95 floor-rescale and leaves the panel 97% empty) survives untouched at 106 features. - Aggregation is never rewritten. Widening a bin must not silently turn max-pooling into mean-pooling. After: legnet 1,048,576 -> 3,987 features (263x), chrombpnet at 2,114 bp unchanged at 106, alphagenome/enformer/borzoi unchanged. tests/test_igv_feature_budget.py checks the budget two ways — the bin-size arithmetic, and end-to-end through the real `_downsample_to_features` with `skip_zeros=False`. That flag matters: callers pass `skip_zeros=not (floor_ok or signed_track)`, and legnet is signed, so zero-skipping was already off and could not have been masking this. The suite also pins ChromBPNet's 20 bp, that max-pooling is preserved, and that no oracle gets a bin narrower than its own resolution — the exact condition that silently floors `bins_per` to 1. 534 passed, 4 skipped, 0 failed (507 + 27 new). Not fixed here: `scripts/rerender_examples.py` is separately broken and I have filed it rather than patch it in this PR — it points at `examples/applications`, renamed to `examples/walkthroughs` in 340f30e, so it dies with FileNotFoundError before doing any work. Fixing only the path is actively harmful: I tried it, and it rewrote 15 shipped reports from MB-scale down to 0.01-0.02 MB because `VariantReport.from_dict` does not carry the per-bin IGV arrays. A crash is safer than silent data loss, so the path stays wrong until the script can preserve what it rewrites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lucapinello
added a commit
that referenced
this pull request
Aug 1, 2026
…ps they found (#134) There was no validation layer at all. `scripts/` holds 8 background builders and 5 regenerators and **0** validators; no test read `examples/`; nothing compared a JSON against the TSV beside it, checked a file size, or noticed a missing provenance stamp. The audit checklist was the validation layer, and it ran when a human ran it. tests/test_committed_examples.py adds six checks, all reading only files already in the repo — no GPU, no network, no weights, no FASTA: - every example_output.json has a TSV beside it - every percentile the TSV reports exists in a JSON in that directory - percentiles are not saturated at the ceiling - every example records when it was generated - every example is reachable from a generator, via AST not grep - no tracked file under examples/ exceeds 20 MiB - plus a source invariant: no generator may write a JSON and no TSV Writing them first found two more real defects, and corrected me twice. TWO DEFECTS FIXED 1. `scripts/regenerate_multioracle.py` wrote example_output.json and .md but never a TSV, so validation/SORT1_rs12740374_multioracle was the only one of 13 directories without one. `rerender_examples.py` refreshes a TSV only `if tsv_path.exists()`, so the gap could not heal itself — and that script is dead anyway (#133). MultiOracleReport has no to_dataframe(), so the TSV is projected from the per-oracle VariantReports with an `oracle` column prefixed: same schema as the single-oracle TSVs, directly comparable. 2. `regen_discovery` assembled its JSON by hand and so carried no analysis_request at all — discovery/SORT1_cell_type_screen was the one example of 13 with no `generated_at`, i.e. precisely the artefact a staleness check could not evaluate. Now stamped like every other. TWO PLACES THE MEASUREMENT CORRECTED THE DESIGN - A `median|quantile| < 0.999` threshold was the obvious guard for staleness and is wrong: SORT1_chrombpnet scores a single track at 0.9995 off a genuine +1.376 effect, so a median test flags a legitimately strong small example. The actual fingerprint of the #119 denominator bug is saturation at *exactly* 1.0 — measured 76-100% on the stale examples versus 0-41% after the refresh. Threshold 0.60, ~20 points of margin either side. - The TSV check was wrong twice before it was right. First it double-counted, because a report stores each score in both `all_scores` and `scores_by_layer` (104 values for 52 rows). Then equality was still wrong: `_variant_report_tsv_rows` writes a deliberate per-layer summary, so region_swap ships 4 TSV rows against 32 scored tracks and TERT 18 against 83. A subset check catches the real drift — SORT1_enformer's TSV described ENCFF571HTM at quantile 1.0 while the JSON beside it described ENCFF430NNH at 0.9605 — and leaves summaries alone. Reachability is parsed with `ast`, not grepped, for a specific reason: a commented-out registry entry is still text, so a grep would happily "find" the three examples that were unreachable for three months. It vanishes from the AST. Follows the tests/test_cherimoya.py:581 idiom because regenerate_examples.py calls parse_args() at module scope and cannot be imported. ARTEFACTS REGENERATED The multi-oracle example, now that #131 and #132 have both landed — regenerating before either would have produced a 131 MB HTML that cannot be pushed. Measured: legnet 131 MB (unpushable) -> 1.78 MB chrombpnet 7.41 MB -> 1.76 MB multioracle 9.47 MB -> 3.95 MB alphagenome 2.99 MB -> 2.99 MB (budget already bound) and its legnet_variant_report.json now carries quantile_score 0.9934 where it previously had no such key — #131 verified in the artefact, not only in a unit test. Largest tracked file under examples/ is now 14.70 MiB against the 20 MiB ceiling. 622 passed, 7 skipped, 0 failed. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
lucapinello
added a commit
that referenced
this pull request
Aug 1, 2026
… signal (#138) The actual root cause of #129, and it is upstream of the rendering code I patched in #132. `_legnet_sliding_prediction` in scripts/regenerate_multioracle.py tiles LegNet's 200 bp window NON-OVERLAPPING (step = win = 200), producing one MPRA-activity scalar per window — then expanded each scalar across its 200 bp span into a length-Q array and declared `resolution=1` with the comment "base-pair resolution, like ChromBPNet". That comment was false. Over a 1,048,576 bp locus it turned 5,243 real numbers into 1,048,576 IGV features, 99.5% redundant, which is what made the report 131 MB and impossible to commit at all (above GitHub's hard 100 MiB). Now it emits the 5,243 values at `resolution=step`. Lossless — the information was always 200 bp — and it removes the redundancy at source instead of relying on a downstream cap to mop it up. MEASURED, and this is the part that matters: the fix RECOVERS signal the #132 feature cap was destroying. features distinct range before (capped) 3,987 2,344 -0.9473 .. +1.1317 after (true 200bp) 5,243 3,633 -1.1333 .. +1.1317 5,243 == ceil(1,048,576/200), i.e. every genuine window. So max-pooling into 50 bp bins had been hiding 1,289 distinct values AND clipping the true minimum (-1.1333 read as -0.9473). File size barely moves: 1.78 -> 1.94 MB. There was no size/resolution trade-off to make here. WHY THE ELEGANT FIX WOULD HAVE BEEN WRONG I had proposed replacing the flat 4,000-feature cap with "never bin below the oracle's native resolution". That rule reads `track.resolution` — the field this commit shows was fabricated as 1 for exactly the track that caused the incident. It would have reinstated the 131 MB report verbatim. The blunt cap worked *because* it ignores that field. So the cap stays as a backstop against redundancy-by-construction, and this commit fixes the thing that made it necessary. Note chorus/oracles/ legnet.py:260 was always correct — it reports `resolution=self.bin_size`; only the regeneration script lied. ARTEFACT CEILING RAISED 20 -> 50 MiB The 20 was mine, picked as "headroom over today's largest" and not derived from any real failure, and it blocked a legitimate 25.70 MB report. Three measurements say a large report is not itself the problem: - these HTMLs gzip to 12-17%, so 25 MB costs ~3.5 MB of history; - IGV.js does not render the payload — it indexes inline features into a red-black interval tree (`qd`/`to`/`io`), culls to the visible window, then summarises to one value per screen pixel (`Pd`, always reached because chorus sets windowFunction). Draw cost is bounded by viewport width, not feature count. The cost is at load, once; - GitHub's 100 MiB is the only hard wall, and 50 MiB is its own advisory threshold — the right place for ours. The ceiling now exists to catch a pathology, not to police size. Losing resolution to hit a number is the wrong trade, which is precisely the mistake this commit undoes. Still open, filed separately: the payload is ~94% scaffolding. Measured on the 14.70 MiB discovery report, 58 of 60.8 bytes per feature are `{"chr":...,"start":...,"end":...,"value":` and all 60 wig tracks are perfect regular grids, so a values-only array plus a (chr,start,step) header is 12.8x smaller with zero loss. That is #135's proper fix and a larger change than this one. 626 passed, 8 skipped, 0 failed. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #129.
rs12740374_SORT1_legnet_report.htmlregenerated at 131 MB (from 1.29 MB) and the consolidated multi-oracle report at 139 MB — both above GitHub's hard 100 MiB per-file limit, so neither could be committed in any form. That blocked regenerating the multi-oracle example at all.Root cause: a missing cap, not tiling
_calculate_track_bin_sizereturned the bareresolutionfor LegNet, which makesbins_per = max(1, bin_size // resolution)equal 1 inside_downsample_to_features— one JSON feature per input bin, no downsampling whatever. Every other oracle fell through to a 3,000-feature budget.Features per track, measured before this change:
Three oracles were unbounded, not just LegNet — LegNet was simply the one that got wide enough to break the limit.
The fix
Keep each oracle's preferred bin size and aggregation, then apply
_MAX_FEATURES_PER_TRACK = 4_000as a floor on the bin width for everyone. Two properties matter:After: LegNet 1,048,576 → 3,987 features (263×), ChromBPNet at 2,114 bp unchanged, alphagenome/enformer/borzoi unchanged.
The test
tests/test_igv_feature_budget.pychecks the budget two ways — the bin-size arithmetic, and end-to-end through the real_downsample_to_featureswithskip_zeros=False.That flag matters. Callers pass
skip_zeros=not (floor_ok or signed_track), and LegNet is signed, so zero-skipping was already off and could not have been masking this. Worth stating because it also means this is independent of #126 — fixing the LegNet percentile does not cause the blow-up, and the blow-up is live today.It also pins ChromBPNet's 20 bp, that max-pooling is preserved, and that no oracle gets a bin narrower than its own resolution — the exact condition that silently floors
bins_perto 1.534 passed, 4 skipped, 0 failed(507 + 27 new).One thing I found and deliberately did not patch here
scripts/rerender_examples.pypoints atexamples/applications, renamed toexamples/walkthroughsin 340f30e — so it dies withFileNotFoundErrorbefore doing any work, and has since the rename. It is the only file in the repo still using the old path.Fixing only the path is actively harmful. I tried it: it rewrote 15 shipped reports from MB-scale down to 0.01–0.02 MB, because
VariantReport.from_dictdoes not carry the per-bin IGV arrays. I reverted all of it. A crash is safer than silent data loss, so the path stays wrong until the script can preserve what it rewrites — filed separately.This also means the TSV-writing I added to that script in #130 is inert, and the multi-oracle TSV gap is still open.
🤖 Generated with Claude Code