Skip to content

Score expressions are the single source of truth (3.1.0) - #291

Merged
iskandr merged 1 commit into
mainfrom
single-default-combined-score-expr
May 13, 2026
Merged

Score expressions are the single source of truth (3.1.0)#291
iskandr merged 1 commit into
mainfrom
single-default-combined-score-expr

Conversation

@iskandr

@iskandr iskandr commented May 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Collapses three nested layers of "default formula lives in two places" into a single mechanism — DSL string expressions — per layer. After this PR, there is no production code path that runs a hardcoded Python score formula while a user-supplied expression governs elsewhere.

Layer 3 — Combined score (vaccine peptide)

  • Deletes the combined_score_mode enum (sqrt_reads_times_epitope / reads_times_epitope / epitope_only).
  • VaccineConfig.combined_score_expr: str now defaults to DEFAULT_COMBINED_SCORE_EXPR = \"sqrt(n_alt_reads) * target_epitope_score\" — the canonical legacy formula, expressed once as a DSL string.
  • VaccinePeptide.__post_init__ resolves None to that default and pre-parses; combined_score always evaluates the AST. No mode branch, no Python fallback.

Layer 2 — Per-epitope total

  • New CandidateEpitope.per_allele_scores: dict[str, float] populated at predict time. epitope_score is a property summing it.
  • VaccinePeptide.target_epitope_score / self_epitope_score now read e.epitope_score directly — no recomputation from raw IC50.
  • _legacy_score_one is removed from production. A test-only oracle lives at tests/_legacy_score_reference.py and is only consumed by parity assertions.

Layer 1 — Per-(peptide, allele) score

  • Deletes _default_score_node (the imperative DSLNode builder).
  • Adds DEFAULT_AFFINITY_SCORE_EXPR_TEMPLATE / DEFAULT_PERCENTILE_SCORE_EXPR_TEMPLATE — string templates that default_score_expr(cfg) formats against EpitopeConfig's scalar threshold fields.
  • build_score_node(cfg) always parses a string (user expression or formatted default). Default and override travel the same code path.

Breaking config change

YAML files setting vaccine_peptides.combined_score_mode no longer parse. Migration:

Old mode value New combined_score_expr
epitope_only \"target_epitope_score\"
reads_times_epitope \"n_alt_reads * target_epitope_score\"
sqrt_reads_times_epitope \"sqrt(n_alt_reads) * target_epitope_score\" (this is the new default — omit to use it)

Verification

Test plan

  • Full pytest suite green (830 passed, 1 skipped)
  • Lint clean
  • HCC1395 end-to-end smoke matches 3.0.2 output byte-for-byte
  • Reviewer confirms migration table is enough for users with combined_score_mode in their YAML

🤖 Generated with Claude Code

Three layers of "default lives in two places" collapsed to one DSL
expression per layer. No code path silently runs a hardcoded Python
formula while a user expression governs elsewhere.

Layer 3 — Combined score:
- Delete ``combined_score_mode`` enum and the three legacy modes.
- ``VaccineConfig.combined_score_expr: str`` defaults to
  ``DEFAULT_COMBINED_SCORE_EXPR = "sqrt(n_alt_reads) * target_epitope_score"``.
- ``__post_init__`` always pre-parses the expression; ``combined_score``
  always evaluates the AST. No mode branch, no fallback.

Layer 2 — Per-epitope total:
- New ``CandidateEpitope.per_allele_scores`` (dict[allele -> score])
  populated at predict time. ``epitope_score`` property sums it.
- ``target_epitope_score`` / ``self_epitope_score`` now read
  ``e.epitope_score`` directly. No re-scoring inside VaccinePeptide.
- ``_legacy_score_one`` deleted from production. Test-only oracle
  moved to ``tests/_legacy_score_reference.py`` for parity assertions.

Layer 1 — Per-(peptide, allele) score:
- Delete ``_default_score_node`` Python builder.
- Two string templates (``DEFAULT_AFFINITY_SCORE_EXPR_TEMPLATE`` /
  ``DEFAULT_PERCENTILE_SCORE_EXPR_TEMPLATE``) formatted against the
  scalar thresholds in ``EpitopeConfig`` and parsed via the topiary
  DSL. ``build_score_node`` always parses a string.
- ``predict_epitopes`` writes the DSL-computed score onto each
  CandidateEpitope's ``per_allele_scores`` so VaccinePeptide can read
  it back without recomputation.

Verified byte-identical rank table on the HCC1395 smoke run vs 3.0.2;
830 tests pass.

Breaking config change: YAML files setting
``vaccine_peptides.combined_score_mode`` no longer parse — switch to
``combined_score_expr`` with one of:
- ``"target_epitope_score"``             (was ``epitope_only``)
- ``"n_alt_reads * target_epitope_score"`` (was ``reads_times_epitope``)
- ``"sqrt(n_alt_reads) * target_epitope_score"`` (default, was
  ``sqrt_reads_times_epitope``)
@iskandr
iskandr merged commit 12ea909 into main May 13, 2026
10 checks passed
@iskandr
iskandr deleted the single-default-combined-score-expr branch May 13, 2026 18:42
iskandr added a commit that referenced this pull request May 13, 2026
The 3.1.0 single-mechanism refactor (#291) moved per-(peptide, allele)
scoring to the DSL and stored the result on
``CandidateEpitope.per_allele_scores``. ``predict_epitopes`` was
updated to populate it; the external-input loaders (``load_lens`` /
``load_pvacseq``) were not.

Result: every external-loaded epitope had ``epitope_score == 0``, so
``target_epitope_score`` was 0, ``combined_score`` collapsed to 0,
and ranking dropped every variant as "No epitopes for peptide." End
users running ``vaxrank --input-pvacseq`` or ``--input-lens`` got
empty reports.

Caught with a real-world smoke against cAIrn's HCC1395 pVACseq TSV
(317 rows in, 0 variants ranked before the fix; 23 ranked after).

Fix: add ``attach_per_allele_scores(epitopes, cfg=None)`` to
``epitope_dsl`` — runs the same DSL pipeline the upstream path uses,
on the loaded epitopes, and stamps each one's per_allele_scores.
Call it at the end of both loaders. Default ``EpitopeConfig()`` so
the loader path's default scoring matches pre-3.1 semantics
exactly. Two regression tests pin both loaders.

832 tests pass, lint clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant