Write scores as a score directory - #416
Merged
Merged
Conversation
luciaquirke
force-pushed
the
feat/magic-score-dirs
branch
from
August 6, 2026 11:35
bb38a46 to
d845688
Compare
luciaquirke
changed the base branch from
feat/magic-per-token-per-query
to
main
August 6, 2026 11:35
MAGIC wrote a bare tensor to scores.pt, so every consumer had to recover from the shape what a score directory simply states. A 2-D tensor is [docs, seq_len] or [docs, queries] depending only on how the run was configured, which is why load_attribution_scores had to read config.yaml, fall back to the rank when there was none, and why scores_are_per_token needed the same treatment. The tensor was never self-contained anyway: it already leaned on config.yaml to disambiguate and on a doc_ids.pt sidecar to map shuffled chunks back to documents. Write the same score directory the scoring pipeline writes. info.json records attribute_tokens and num_scores, so both questions are answered by the store, and the reader collapses to: load it, negate if higher_is_better, report num_scores > 1. Per-token scores pack into the ragged token store exactly. The weight grid is [rows, seq_len] and weighted_causal_lm_ce reads example_weight[:, :-1], so a row's columns past length - 1 never receive gradient — precisely the rows compute_num_token_grads allocates. to_grid then restores the same width, since max(length - 1) + 1 is the seq_len the grid started with. The round-trip is exact in shape and value, tested both ways. doc_ids moves into the directory as doc_ids.npy, in the grid's shape, so a per-token store carries everything needed to aggregate per document. Deleted along the way: the .pt branch of load_attribution_scores, the config lookup and shape fallback behind it, scores_are_per_token's two non-directory branches, and read_first_step_config, whose only caller was that lookup. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The old name said what was loaded but not in which orientation, and the orientation is the part a caller gets wrong. Scores come back in the loss-diff convention: higher_is_better stores are negated on load, and validate correlates them against baseline - loss, so a proponent lands negative. The name now carries that, and the docstring states it outright. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
luciaquirke
force-pushed
the
feat/magic-score-dirs
branch
2 times, most recently
from
August 6, 2026 12:54
c976dee to
dc5af7f
Compare
It is a thin sign-normalising wrapper over load_scores, which lives in data alongside the store format it reads — Scores, offsets, to_grid — so the two belong together. Nothing about it is validation-specific; magic imported it from validate only because that is where it happened to sit. No new dependency edge: data already imports from config.config, and the config layer does not import data, so ScoreConfig and load_subconfig come along without a cycle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
luciaquirke
force-pushed
the
feat/magic-score-dirs
branch
from
August 6, 2026 12:55
dc5af7f to
ad09dba
Compare
Deleting the .pt branch made every run finished before score directories
unreadable, and not with a message: load_scores(Path("scores.pt")) opens
scores.pt/info.json and raises NotADirectoryError.
Restore the read path only, behind a suffix check, with the ambiguity
resolution it had: a 2-D tensor is per-token unless the run config beside
it records query_method: none. Marked for removal in December 2026.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # tests/test_per_query_magic.py
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.
Problem
MAGIC wrote a bare tensor to
scores.pt, so every consumer had to recover from the shape what a score directory simply states. A 2-D tensor is[docs, seq_len]or[docs, queries]depending only on how the run was configured — which is whyload_attribution_scoreshad to readconfig.yaml, fall back to the rank when there was none, and whyscores_are_per_tokenneeded the same treatment.And the tensor was never self-contained anyway. It already leaned on
config.yamlto disambiguate its own axes and on adoc_ids.ptsidecar to map shuffled chunks back to documents. It wasn't a simpler format than a score directory — it was an undocumented one.Fix
Write the same score directory the scoring pipeline writes.
info.jsonrecordsattribute_tokensandnum_scores, so both questions are answered by the store, and the reader collapses to: load it, negate ifhigher_is_better, reportnum_scores > 1.Deleted: the
.ptbranch ofload_attribution_scores, the config lookup and shape fallback behind it,scores_are_per_token's two non-directory branches, andread_first_step_config, whose only caller was that lookup.Why the ragged store fits exactly
The one thing that made this non-obvious. MAGIC's weight grid is dense
[rows, seq_len]; the token store is ragged,length - 1values per row. They coincide, and not by luck:weighted_causal_lm_cereadsexample_weight[:, :-1], so a row's columns pastlength - 1never receive gradient — precisely the rowscompute_num_token_gradsallocates.to_gridrestores widthmax(length - 1) + 1, which is theseq_lenthe grid started with, for bothchunk_length > 0and unpacked data.So packing drops only structural zeros and the round-trip is exact in shape and value.
test_save_magic_scores_round_trips_the_gridasserts that for single- and multi-query.doc_idsMoves into the directory as
scores/doc_ids.npy, in the grid's shape, so a per-token store carries everything needed to aggregate per document rather than depending on a sibling file.Output layout
Verified end to end on CPU for all three modes — per-token
(5, 8), per-doc(5, 1), per-token per-query(5, 8, 2)— each loading back at the shape MAGIC used to write, withmulti_queryandscores_are_per_tokenreported frominfo.json.Testing
110 passed, 38 skipped across every test file touching magic, validate or scores. Tests that covered the removed
.ptpath are gone; those that readscores.ptnow read the directory, includingtest_distributed_magic.py.docs/magic.rst"Output files" rewritten, sphinx clean.pre-commit --all-filesclean.🤖 Generated with Claude Code