alphagenome_pt: assay_ids=None crashed, and the splice head was being read as logits - #239
Merged
Merged
Conversation
…as logits
`predict_variant_effect(..., assay_ids=None)` raised `invalid literal for int() with base 10: 'logits'`
on this backend while an explicit list of ATAC ids worked. The default is all 5,168 tracks, and the 4
SPLICE_SITES tracks are always among them, so every default-argument call failed.
**The cause: head-output dicts are keyed by two unrelated things.** Most heads key by resolution
({1: ..., 128: ...}); SpliceSitesClassificationHead keys by tensor kind ({"logits", "probs"}). The
extraction loop reassigned its own `res` variable to whichever key it picked and then called
`int(res)`, so a splice track reached `int("logits")`. Resolution and tensor selection are now separate
questions, asked separately, in `_select_head_tensor` and `_as_resolution`.
**Fixing the crash exposed a second, quieter bug.** The old fallback took `next(iter(head_out.keys()))`,
which for `{"logits": ..., "probs": ...}` is dict insertion order -- i.e. **logits**. The JAX reference
returns `{'logits': logits, 'predictions': softmax(logits)}` and treats 'predictions' as the
prediction: alphagenome_research/model/model.py:281 reads exactly that key to derive splice junctions.
The pt port names the same tensor 'probs'. So the two backends would have disagreed on those 4 tracks
-- unbounded logits against probabilities in [0, 1] -- had the crash not masked it first. Selection now
prefers 'probs' explicitly and no longer depends on insertion order.
Non-numeric resolutions degrade to 1 bp with a warning rather than raising, since a hard failure there
is what produced the original user-visible break.
13 tests, and 6 of them fail against the pre-fix logic (verified by reverting): the crash half via
`_as_resolution`, the correctness half via probs-vs-logits and an insertion-order test that pins the
answer against both dict orderings. They exercise the two functions the production loop calls rather
than a reimplementation, and need no torch or GPU because the tensors are opaque to the selection
logic. That isolation is deliberate -- the guard written for Sei's equivalent `assay_ids=None` defect
asserted at the validation layer and did not catch this one, which lives in the execution path.
Fast suite 2,136 passed / 35 skipped (2,123 + 13).
Two adjacent defects found while probing, NOT fixed here and not regressions from this change:
* `heads=` filtering breaks when splice_sites is requested with a partial head set --
`predict(..., ['ATAC/...', 'SPLICE_SITES/donor/+'])` dies with "Expected size for first two
dimensions of batch2 tensor to be: [8, 16] but got: [8, 1]". The full head set works, which is why
assay_ids=None gets far enough to hit the bug above.
* The JAX backend currently fails to load on this host: "'alphagenome/transformer_tower/mha_block/
linear_embedding/w' with retrieved shape (1536, 1536) does not match shape=[24576, 1536]". It loaded
fine earlier the same session, so this looks environmental rather than a code regression.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ond copy `predict_variant_effect(..., assay_ids=None)` still raised `invalid literal for int() with base 10: 'logits'` end-to-end after the previous commit, while all 13 unit tests passed. Cause: `use_environment=True` — this backend's **default** — does not call `_predict_raw`. It executes `alphagenome_pt_source/templates/predict_template.py` inside the per-oracle conda env, and that template carried its own copy of the same six lines, bug included. The fix corrected the in-process path only, which is the path this backend does not use by default. Every unit test passed because they tested the function I had fixed. The template now imports `_select_head_tensor` and `_as_resolution` instead of restating them. Two copies of one rule is the exact failure mode this repo keeps rediscovering — the Sei normalizer and the Jupyter kernel were both correct code that nothing called, and the Sei background builder nearly shipped a second copy of the nucleosome correction for the same reason. Three new guards, all three verified to fail against the un-fixed template: the template must import the helpers, must not contain `int(res)`, and must not reimplement the `next(iter(head_out.keys()))` fallback. Source-level assertions, because the template runs in a conda env this test process cannot import torch from. 16 tests total now. Note the `cache_key = (pt_key, res)` below the change is still correct: selection is deterministic given (head_out, res), so the key still uniquely identifies the tensor. Previously the key held "logits" for splice heads; now it holds 1, which is the track's real resolution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CHANGELOG moves the entry from Known issues to Fixed, covering all three parts: the crash, the
splice-site tracks having been read as logits rather than probabilities, and the env-mode template
holding a second copy of the same logic so the first fix left the default path broken.
The audit report gets the same correction plus the sharper version of its own lesson. It already noted
that the Sei guard failed by asserting at the wrong *layer* (validation rather than execution). This adds
that a test can also fail by asserting against the wrong *copy* -- 13 unit tests passed here while the
path users actually take stayed broken, because env mode runs a template rather than the method I fixed.
Two claims from earlier in this investigation, both since checked and both wrong:
* "The JAX backend fails to load on this host" (weight shape mismatch). It loads fine on a re-run, so
that was transient. Not carried into any committed document.
* "predict() is broken for these tracks." It is not. Instrumenting the template showed len(sequence)=24
reaching the model, and len("chr1:109274468-109275468") == 24: predict() takes a str as literal DNA,
which all four coordinate-taking oracles do identically. My probe passed a coordinate string where the
API expects bases. predict() with a tuple works.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ow complete 2/2 pairs, max|diff| 0.000e+00 over 9 tracks spanning all 9 heads (288 values), env mode, GPU 6, runs sequential and alone. This was the last unmeasured row in #238's table, and it stayed unmeasured because the assay_ids=None crash fixed in this PR blocked it. With it closed, all nine oracles are measured cross-process and **Enformer is the only one that drifts** -- which is the strongest form of the argument for accepting that drift rather than rebuilding the nulls: ChromBPNet, the other TF-backed oracle, is bit-exact, so the accepted risk is one oracle wide and near_ties_at_cutoff already covers the one way it reaches a reader. Worth recording how the gap read: "not measured" looked like a scheduling gap in a table, when it was actually a live bug in the default code path. The table's own blank was evidence. Updates the focused audit report, the F8 localisation write-up, and the CHANGELOG. Fast suite 2,139 passed / 35 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…es today Each of these had me confidently blaming the wrong thing during the alphagenome_pt investigation, and each is invisible in the error message you actually get. * `predict()` takes a str as literal DNA; predict_variant_effect takes the same string as a region. So a coordinate string one-hot encodes as 24 non-ACGT characters and dies inside the model with "Expected size for first two dimensions of batch2 tensor to be: [8, 16] but got: [8, 1]" -- an error naming neither the sequence nor its length. I attributed that to the heads= filter, then to interval length, before instrumenting the template and finding len(sequence)=24. * use_environment=True runs a template, not the method you edited. A fix to the in-process path can leave the default path broken with every unit test green. That is exactly what happened in this PR. * The child process sees your working tree, so probing from the wrong branch reproduces the original bug and looks like the fix failing. (This one bit the Sei work earlier in the session too.) Plus the mechanics: Bash calls cap at 10 minutes, which model load plus a forward pass routinely exceeds, and an unbuffered grep leaves the output file empty until exit -- both of which made working probes look like hangs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lucapinello
added a commit
that referenced
this pull request
Aug 18, 2026
…against a sigmoid-space null (#240) * alphagenome_pt: 734 SPLICE_SITE_USAGE tracks were returning log-space values Third round on one bug. #239 fixed 4 tracks and missed 734, because the upstream port names the activated tensor differently in each head: `probs` in SpliceSitesClassificationHead, `predictions` in SpliceSitesUsageHead (plus a track_mask entry). #239 hardcoded "probs", so the usage head's 734 tracks kept falling through to raw logits. Measured against JAX on the same interval: PyTorch -13.8..-11.3 where JAX gives 1e-6..1.2e-5, with exp(logits) recovering the JAX values to 2.7%. Across the full 1 MB SORT1 window the two backends correlated at **0.20** for this head. **This was worse than wrong numbers.** alphagenome_pt has no background null of its own -- _CDF_ALIASES maps it to alphagenome, justified by a comment asserting the two produce identical predictions -- so those log-space values were ranked against a sigmoid-space reference population. Percentiles for the affected tracks were not noisy, they were meaningless. alphagenome_pt is also in _MULTI_TRACK_ORACLES, so a discovery run enumerates all 738 affected tracks. Same failure mode as Sei's stale null earlier in this session, reached from the opposite direction: there the null drifted from the predictions, here the predictions drifted from the null. No committed artefact is affected -- both example outputs containing SPLICE_SITE_USAGE were produced by the JAX backend. Checked, not assumed. Selection now tries ("predictions", "probs") in order rather than one hardcoded name, never falls back to logits or to a *_mask entry while any activated key exists, and warns loudly when it cannot find one. A silent fall-through to logits is invisible downstream: log-space values are still finite, still float32, still the right shape. **Extends the test that let all three rounds happen.** test_jax_pt_chorus_api_equivalence_at_sort1 is what makes the _CDF_ALIASES premise checkable, and it compared three DNASE tracks -- one head, 1 bp, bare tensor -- so it was structurally incapable of seeing a dict-keyed head. A guard aimed at "do the backends agree" that samples the easiest 3 of 5,168 tracks reads as coverage and is not. New test covers one track per output type, derived from metadata so it keeps covering every head if ids move. Bounds from full-array measurement: correlation > 0.99 (measured 0.9998-1.0000) and peak-relative < 8% (worst head 4.945%), which is loose against bf16 noise but seven orders of magnitude tighter than the defect. The tight 2% bound on the three DNASE tracks is left untouched rather than loosened. Verified: fast suite 2,142 passed / 35 skipped; 19 unit tests with 4 failing against the pre-fix rule; both integration equivalence tests passing on the real window (2 passed in 386.91s); and the new integration guard run against the reverted rule, where it fails on correlation 0.2005. Worth recording: three sequential fixes for one bug, each verified against the case that motivated it rather than the class of case. Enumerating every dict-returning head in the port took one command and would have collapsed all three rounds into one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Record why this bug could only exist on the PyTorch side Structural, and it says where to look next rather than just what went wrong. The JAX backend never touches a raw head dict: its template asks the library for an assembled result (output.get(ot_enum).values), and the alphagenome library is what selects 'predictions' out of each head's {'logits', 'predictions'}. The activation choice is made upstream, so the JAX path is correct by construction -- verified, not assumed: measured JAX values sit in [0,1] for both splice heads. The PyTorch port exposes raw head outputs, so chorus itself has to decide which tensor is the prediction. That is a responsibility the JAX backend delegates to the library and the pt backend cannot. Every divergence found in this session lived in exactly that gap. The rule that follows: wherever chorus reimplements something the JAX library does for the other backend, assume divergence until measured. Also sweeps all nine oracles for the same oracle-vs-template duplication shape that hid this bug. Mostly benign boilerplate (device selection, one-hot encoding, retry sleeps), with two worth knowing: chrombpnet defines multinomial_nll in three places (a Keras custom_objects entry, so divergence fails loudly at load rather than changing numbers silently), and both alphagenome backends duplicate the resolution lookup -- the pt side now routes through the shared helper, the JAX side appends info["resolution"] raw, which is safe only because its metadata is always numeric. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <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
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.
Fixes the defect recorded as a known issue in #238, plus a second one that fixing the first exposed, plus
a third that only showed up end-to-end.
The crash
predict_variant_effect(..., assay_ids=None)raisedinvalid literal for int() with base 10: 'logits'on this backend, while an explicit list of ATAC ids worked. The default is all 5,168 tracks and the 4
SPLICE_SITEStracks are always among them, so every default-argument call failed.Cause: head-output dicts are keyed by two unrelated things. Most heads key by resolution
(
{1: ..., 128: ...});SpliceSitesClassificationHeadkeys by tensor kind(
{"logits": ..., "probs": ...}). The extraction loop reassigned its ownresvariable to whichever keyit picked, then called
int(res). Resolution and tensor selection are now separate questions, askedseparately, in
_select_head_tensorand_as_resolution.The quieter bug underneath it
The old fallback was
next(iter(head_out.keys()))— dict insertion order, which for{"logits", "probs"}is logits.The JAX reference returns
{'logits': logits, 'predictions': softmax(logits)}and treats'predictions'as the prediction:
alphagenome_research/model/model.py:281reads exactly that key to derive splicejunctions. The pt port names the same tensor
probs. So the two backends would have disagreed on those4 tracks — unbounded logits against probabilities in [0, 1] — had the crash not masked it. Selection now
prefers
probsexplicitly and no longer depends on insertion order.The third one: my first fix was incomplete, and the unit tests did not notice
After the first commit, all 13 tests passed and
assay_ids=Nonestill crashed end-to-end.use_environment=True— this backend's default — does not call_predict_raw. It executesalphagenome_pt_source/templates/predict_template.pyin the per-oracle conda env, and that templatecarried its own copy of the same six lines. I had fixed the path this backend does not use by default.
The template now imports the two helpers instead of restating them. Two copies of one rule is the failure
mode this repo keeps rediscovering — the Sei normalizer and the Jupyter kernel were both correct code
nothing called, and the Sei background builder nearly shipped a second copy of the nucleosome correction.
Tests
16 tests, and 9 of them fail against the pre-fix code (verified by reverting each part):
_as_resolution, the correctness half via probs-vs-logitsand an insertion-order test that pins the answer against both dict orderings.
int(res), must not reimplement thenext(iter(head_out.keys()))fallback.The oracle tests need no torch or GPU because the tensors are opaque to the selection logic. That
isolation is deliberate but not sufficient on its own, and this PR is the proof: the guard written for
Sei's equivalent
assay_ids=Nonedefect asserted at the validation layer and did not catch this one, andmy own unit tests here passed while the default path stayed broken. Hence the source-level template
guards and the end-to-end run below.
Fast suite 2,139 passed / 35 skipped.
End-to-end verification
predict_variant_effectin env mode over 9 ids spanning all 9 heads, includingSPLICE_SITES/donor/+:9 tracks returned, no crash. The 128-bp heads still return 8,192 bins against 1,048,576 for the 1-bp
heads, so
_as_resolutionreports real resolutions rather than defaulting.Correction to an earlier version of this description
An earlier revision claimed
heads=filtering was broken in the in-process path, on the strength of apredict()call dying withExpected size for first two dimensions of batch2 tensor to be: [8, 16] but got: [8, 1]. That claim was wrong and is withdrawn. I then blamed interval length, which was alsowrong — it reproduced at 1 kb and at full window, in-process and in env mode.
Instrumenting the template to log what actually reached the model gave the answer:
len("chr1:109274468-109275468") == 24.predict()takes astras literal DNA, so my coordinatestring was one-hot encoded as 24 non-ACGT bases. My probe misused the API; the heads filter and the
interval were never involved. All four coordinate-taking oracles behave identically here, so it is
intended and uniform.
The one residual, pre-existing trap worth a follow-up: the same string is a valid region for
predict_variant_effectand literal DNA forpredict(), and the mistake surfaces as a torch shapeerror rather than "this does not look like DNA". Out of scope here — it is uniform across oracles and
predates this change — but it cost me real time, which is the argument for validating it.
This also closes #238's last open row
With the crash fixed,
alphagenome_ptcross-process determinism is finally measurable, and it isbit-exact: 2/2 pairs,
max|diff| = 0.000e+00over 9 tracks spanning all 9 heads (288 values).So all nine oracles are now measured cross-process and Enformer is the only one that drifts. That is
the strongest form of the argument for accepting the drift rather than rebuilding the nulls: ChromBPNet,
the other TF-backed oracle, is bit-exact, so the risk is one oracle wide and
near_ties_at_cutoffalready covers the one way it reaches a reader.
Worth noting how the gap read: "not measured" looked like a scheduling gap in a table when it was
actually a live bug in the default code path. The blank was the evidence.