Repository navigation
Replace mokapot rescoring engine with ristretto - #268
Merged
Merged
Conversation
Swap ms2rescore/rescoring_engines/mokapot.py for a new ms2rescore/rescoring.py built on ristretto's rescore()/evaluate(). Update config schema and defaults for ristretto's train_fdr, and drop lower_score_is_better in favor of automatic score-direction inference. Rework report generation to consume ristretto's before/after RescoreResult objects. Update docs, examples, and add integration tests for the new pipeline.
_write_group_metadata crashed with KeyError for decoy PSMs whenever protein-level FDR used picked-protein competition (id_decoy_pattern set): ristretto's picked_rollup strips the decoy pattern from group keys, so strip it the same way before the lookup. Log a warning when plain rollup runs instead (id_decoy_pattern not set), since protein q-values/PEPs are then less accurate. Replace infer_score_direction's naive mean-target-vs-mean-decoy-score comparison with a competition-based test: run ristretto's spectrum-competed target-decoy evaluation under both direction hypotheses and pick whichever identifies more PSMs at train_fdr. More robust when the raw score is weak or decoys are few, and correctly competes multiple candidate PSMs per spectrum (relevant since this runs before rank filtering). Warn when the inferred direction is lower-is-better, since that's the less common case.
psm.get_usi() bakes the peptidoform into the string. Once rename_to_usi overwrote psm_list's spectrum_id (before rescore()), ristretto's spectrum-grouped competition -- keyed on spectrum_id -- saw every candidate peptidoform for a spectrum as its own singleton group, breaking competition whenever max_psm_rank_input > 1 (default 10) and rename_to_usi was enabled. Add spectrum_usi(), a peptidoform- free (still legal) USI, and use it instead. Since USI rename must stay after spectrum-file matching (which needs the native spectrum_id) and evaluate_before() runs before that point, remap before_result's spectrum_id through the same transform right after it returns, so before/after use consistent spectrum_id strings for the report merge. Add rescoring.rescore()'s report_result: a rank-1-competed result independent of max_psm_rank_output, computed on the pre-trim ML scores when multi-rank output is configured. The report and 1%-FDR console counts now always use rank-1 identifications (before_result/after_report_result), regardless of how many ranks per spectrum the user wants in the real output. Move fix_constant_pep's decoy-removal into rescore() itself, filtering psm_list and final_result together so they can't desync (previously after_result -- used for parquet tables, report, and the 1%-FDR count -- kept the higher-scoring decoys that fix_constant_pep had already dropped from psm_list). Restore the mumble-candidate exclusion (via the original_psm metadata flag) when counting identified PSMs before rescoring, matching what the deleted _log_id_psms_before used to do.
ristretto's RescoreResult schema has no room for a passthrough run column, so attach it after the fact (index-aligned against the original PSMList) on before_result, final_result, and report_result. Enables merging before/after by (run, spectrum_id) in the report, guarding against spectrum_id collisions across multiple input files. write_rescoring_tables now also writes an after_report Parquet table set when it differs from after (i.e. max_psm_rank_output > 1), so standalone report regeneration (ms2rescore-report) can use the same always-rank-1 view as a live run instead of the multi-rank result.
…p levels create_psm_dataframe: filter psm_list to rank==1 (the report is always a single-best-PSM-per-spectrum view, regardless of max_psm_rank_output) and merge before/after by (run, spectrum_id) instead of (spectrum_id, peptidoform). The old peptidoform-keyed merge produced NaN score_before/qvalue_before/pep_before for exactly the PSMs where rescoring promoted a different winning peptidoform for a spectrum -- the normal, expected effect of rescoring. identification_overlap now takes the before/after RescoreResults directly and compares ristretto's own rollup tables (psms, peptidoforms, peptides, proteins) instead of re-deriving sets from the merged psm_df, which only worked at the peptidoform/peptide/protein level if the winning peptidoform never changed. Renamed "PSM level" to "spectra" and added "peptidoforms" as its own level (previously conflated with "peptides"). ReportData now carries the before/after RescoreResults and a rescoring_tables_unavailable flag, distinguishing "before/after tables missing" from "loaded but zero identifications" -- from_files prefers the after_report table (rank-1 view) over after (which may be multi-rank) when both exist. The overview tab shows an explicit notice when tables are unavailable instead of silently rendering empty stats.
The "Rescoring result tables" table had inconsistent column-divider positions across its border/header/content rows (four different widths), which docutils/Sphinx rejects as a malformed table. Rebuilt it with consistent column widths (verified programmatically: every row shares identical +/| column positions).
fix_constant_pep now takes/returns a (PSMList, RescoreResult) pair; rescore() returns (psm_list, after_result, after_report_result). RescoreResult.psms now always carries a run column. Adds coverage for the new report_result (rank-1 view, distinct from a multi-rank final result) and the after_report Parquet table written when they differ. Also asserts rescoring_tables_unavailable is set when the ristretto Parquet tables are missing.
fix_constant_pep previously only filtered result.psms, leaving peptidoform/peptide/protein rollups computed on the decoy-contaminated population -- _write_group_metadata would then look up stale scores/qvalues/PEPs for surviving PSMs. Now recomputes q-values, PEP, and rollups from scratch via ristretto.evaluate() on the corrected population (falling back to plain filtering if no decoys remain to recompute a competition against). report_result (the separate rank-1 competition used when max_psm_rank_output > 1) never went through this fix at all, so a degenerate constant-PEP=1.0 result could reach the report while the real output looked fine. Split the fix into a result-only core (_fix_constant_pep_result) plus the existing psm_list-aware wrapper, and apply the core check to report_result independently.
…n report compute_id_stats/identification_overlap consumed before_result unmasked, unlike core.py's console-log count, which excludes mumble-generated alternate candidates via psm.metadata['original_psm']. Attach that mask onto before_result.psms itself (evaluate_before), the same way run already is, so every consumer (report stats, identification overlap, and a from_files regen via the persisted parquet column) sees a consistent "before" baseline. Simplifies core.py's n_id_before to read the attached column instead of recomputing the mask separately. identification_overlap's "spectra" level still grouped by bare spectrum_id, inconsistent with the rest of the report pipeline (which already merges by (run, spectrum_id) to guard against collisions across input files). Generalized the level-comparison loop to accept a compound key and use (run, spectrum_id) for that level. Adds direct unit coverage for both: _n_identified's masking behavior, and identification_overlap's run-based disambiguation (which the old bare-spectrum_id grouping would undercount).
write_rescoring_tables() writes an additional set of *_after_report Parquet tables when max_psm_rank_output > 1 (the rank-1 report view, distinct from the multi-rank *_after files), which wasn't mentioned anywhere in the output-files reference.
build_features_dataframe now includes a "run" column, and every ristretto.rescore()/evaluate() call passes run_col="run" (requires ristretto-ms>=0.3.0). Competition and CV splitting now group by (run, spectrum_id) at the source, instead of spectrum_id alone -- previously, two different physical spectra from different input files sharing a native spectrum ID (e.g. raw scan numbers restarting per file) were silently treated as one competition group. Also fixes the same collision in rescore()'s max_psm_rank_output>1 rank-trimming step, which grouped by bare spectrum_id too. Removes the _attach_run() post-hoc workaround entirely: "run" now comes back natively on every RescoreResult.psms from ristretto itself, so there's no more index-alignment hack needed on the ms2rescore side. Bumps the ristretto-ms pin to >=0.3.0.
max_psm_rank_output is a special feature for surfacing ambiguous results (e.g. mumble's multiple candidate peptidoforms per spectrum), not for statistically rigorous FDR control when > 1 -- that trade-off is now explicit and applied consistently everywhere, instead of computing a separate always-rank-1 "report view": - rescore()/evaluate_before() share one trim-and-evaluate helper (_trim_and_evaluate): rank==1 competes properly (ristretto's real spectrum competition); rank>1 keeps the top-N by score per spectrum, then computes q-values/PEP over that population as independent rows. Documented as not statistically rigorous in that mode. - evaluate_before() now mirrors max_psm_rank_output (was hardcoded to rank-1), so before/after counts stay comparable at the same depth, and excludes mumble-generated candidates from the population itself (metadata["original_psm"] is False) rather than just masking the count afterwards -- they're ms2rescore's own addition, not part of the original search engine's baseline. - rescore() drops the separate report_result/after_report entirely; returns a single (PSMList, RescoreResult) again. - Report charts that need a single row per spectrum (score scatter, FDR ECDF comparison) now collapse best-per-(run, spectrum_id) themselves, from the before/after RescoreResults directly, instead of relying on a blanket rank==1 filter upstream. identification_ overlap's set-based counting already handled multi-rank spectra correctly (a set of (run, spectrum_id) keys can't double-count). Rescoring result tables move from Parquet to TSV -- a user-facing deliverable (biologists read these directly, e.g. in Excel), separate from the full PSM list output. Only "after" is written; "before" is fully reconstructable from each PSM's provenance_data, so it's never persisted. Standalone report regeneration (ms2rescore-report) no longer reads any separate rescoring-result file at all: it rebuilds both before and after RescoreResults directly from the main PSM file (evaluate_before_from_provenance, evaluate_after_from_psm_list). Along the way: fixed two real bugs this surfaced -- psm_utils.PSM.metadata/provenance_data are typed dict[str, str], so a value written to TSV and read back is a string, not the original bool (a literal "False" string is truthy in Python) -- original_psm checking and provenance score parsing are now robust to that. Also, PSMList's boolean-mask indexing returns the same underlying PSM object references, not copies, so evaluate_before_from_provenance now takes a real deep copy before mutating scores, to avoid corrupting the caller's psm_list.
Rescoring result tables are now plain TSV (not Parquet) at every
identification level, and only the post-rescoring ("after") result is
written -- "before" is reconstructed on demand from the main PSM
list's provenance data, so it's never persisted. Also documents the
max_psm_rank_output > 1 caveat (multi-rank rows, not statistically
rigorous FDR) and drops the now-obsolete after_report paragraph.
ristretto-ms>=0.3.0 is now on PyPI, so the local ../ironbrew editable path override is no longer needed. Removes the now-empty [tool.uv.sources] override and its TODO, and regenerates uv.lock to resolve ristretto-ms from the registry.
Add rescoring.model config option (svm, the default iterative Percolator-style SVM; or lda, a single-pass Fisher LDA that's faster but less powerful on hard-to-separate datasets), threaded through to ristretto.rescore()'s model kwarg. Adds a GUI dropdown alongside the existing training-FDR field. Removes write_rescoring_tables: for simplicity, the rescoring result tables are now always written (they're the intended user-facing deliverable, not an optional extra). Drops the config option from the schema/defaults/GUI, and core.py's conditional around writing them. Updates config_schema.md by hand, since the jsonschema2md CLI's pinned Python interpreter is missing on this machine (pre-existing environment issue, unrelated to this change) -- the generated sections were mirrored exactly by hand instead.
…into feat/add-ristretto
Adds a top-level report_fdr setting (default 0.01) used for console-logged identification counts, HTML report stats/charts, and FlashLFQ output filtering. This is independent of rescoring.train_fdr, which only controls ristretto's semi-supervised training. Standalone report regeneration (ms2rescore-report) gains a --fdr option to override the report_fdr stored in the original run's full-config.json, without rerunning rescoring.
…e-ristretto-fixes
Clarifies alongside the existing null (skip rescoring) note that
rescoring: {} is equivalent to omitting the key, since cascade_config
deep-merges schema defaults into a partial dict. Also fixes a stale
mention that rescoring only exposed train_fdr (model was added earlier).
A user-provided rescoring dict (e.g. "rescoring": {} or {"model": "lda"})
is passed through as-is by the config cascade -- same as feature_generators
and psm_generator, where an override is taken at face value rather than
merged with defaults (CascadeConfig's max_recursion_depth stops it from
reaching into these nested dicts either way). rescoring.py and parse_psms.py
were indexing config["rescoring"]["train_fdr"]/["model"] directly, which
KeyErrors as soon as either key is missing.
Fixed by using dict.get(..., default) where a value is needed before
calling ristretto, and by passing config["rescoring"] to ristretto.rescore()
via **kwargs instead of naming train_fdr/model explicitly, so ristretto's
own defaults (train_fdr=0.01, model="svm") apply to whatever the user's
config omitted.
…nal API surface - Fix rescore() always returning empty feature_weights/n_iterations: it rebuilds the final result via ristretto.evaluate() (no training, so those fields come back empty), discarding the real values from the earlier ristretto.rescore() call. Now carried over explicitly, including through the constant-PEP fix path. Regression test added. - Drop the "ristretto"/"rescore" infix from rescoring-table filenames (<prefix>.psms.tsv, .peptidoforms.tsv, .peptides.tsv, .proteins.tsv, .weights.tsv) and rename the main PSM list output from <prefix>.psms.tsv to <prefix>.tsv, freeing up the naming collision. Docs and tests updated. - Remove PIN file output for disabled rescoring/DEBUG mode: the main PSM list TSV already carries all rescoring features, and PIN/Percolator format is no longer relevant since the ristretto migration. - Simplify USI handling: build the (run, spectrum_id) -> USI lookup once and reuse it for both the early before_result remap and the later psm_list rename, instead of generating USIs twice. Also fixes a latent bug where the two passes used mismatched row indices when mumble is enabled. Inlined the one-line USI construction and removed the now-unused spectrum_usi helper. - Report text: chart descriptions in texts.toml hardcoded "1% FDR threshold", which became wrong once report_fdr became configurable. Now templated with the actual configured value. - Docs: swap the train_fdr example from 0.1% to 10%, more representative of a difficult dataset where the 1% default yields no accepted PSMs; fix a stale ms2rescore-report CLI usage example. - GUI: word-wrap the rescoring model description like other Advanced tab descriptions. - Minimize internal API surface: rename utils.py to _utils.py (mirroring the existing _version.py convention) and merge rescoring.py's functions into it, since none of it is used outside ms2rescore's own submodules -- make that explicit rather than presenting it as public API. Also privatized build_features_dataframe and fix_constant_pep, which were only ever called internally within the rescoring workflow itself.
The GUI's logging setup only ever attached a plain FileHandler for <prefix>.log.txt -- unlike the CLI, it never used a recording Rich console or called save_html(), so <prefix>.log.html (documented as always written) was silently never created for GUI-launched runs. Added the same Console(record=True) + RichHandler + save_html() pattern the CLI already uses, wrapped around the rescore() call so the HTML log is written even if the run raises. Also add an INFO-level log line when rescoring actually starts, since the per-fold/rollup/competition detail ristretto itself logs is moving to DEBUG level (fixed at the source, in ristretto).
Cumulative summary of the mokapot -> ristretto rescoring engine migration (report_fdr, model choice, auto score-direction inference, output-file renames, GUI log.html fix, and the resulting config/API removals), merged into the existing Unreleased section.
Review findings from the cumulative-diff review, addressed: - rescoring: null never actually disabled rescoring (CascadeConfig's none_overrides_value=False silently ignores it), and the "skip rescoring" path that was reachable only in principle would have written no output at all. Rather than fix the cascade-merge bug, remove the skip-rescoring feature entirely: rescoring always runs, the null option is dropped from the schema (now a validation error instead of a silent no-op), and docs updated to match. - infer_score_direction() now groups by run (matching _build_features_dataframe's existing guard), so multi-file input with colliding native spectrum/scan IDs across runs no longer risks corrupting the inferred score direction. Added direct unit tests, including one confirming run_col reaches ristretto.evaluate(). - Simplified the `config["rescoring"].get(...) if config["rescoring"] else 0.01` ternary in parse_psms.py/_utils.py to a plain .get() -- config["rescoring"] can no longer be None after config parsing. - Removed stale mokapot/rescoring_engine definitions from docs/source/config_schema.md (hand-maintained schema reference). Also reformats CHANGELOG.md's "Breaking changes" section (non-standard Keep a Changelog category) into the canonical Added/Changed/Removed sections, with breaking items tagged [BREAKING].
ArthurDeclercq
approved these changes
Jul 17, 2026
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.
Added
rescoringconfiguration option for the ristretto rescoring engine:train_fdrandmodel("svm", default, or"lda", faster but less powerful). Acceptsnullto skiprescoring,
{}for defaults, or a partial dict -- missing keys fall back to ristretto's owndefaults.
report_fdrconfiguration option: FDR threshold used for console-loggedidentification counts, the HTML report's stats/charts, and FlashLFQ output filtering.
Previously hardcoded at 1% throughout.
ms2rescore-reportCLI: new--fdroption to regenerate a report at a different FDRthreshold without rerunning rescoring.
via spectrum-competed target-decoy evaluation, replacing the user-set
lower_score_is_betteroption.
<prefix>.psms.tsv,.peptidoforms.tsv,.peptides.tsv,.proteins.tsv,.weights.tsv) are now always written as plain TSV, independent of rescoringengine internals.
ristretto-msdependency: a lean, dependency-light (numpy/scikit-learn/pandas)reimplementation of the Percolator semi-supervised rescoring algorithm.
svm/lda); GUI runs now also write an HTML log file(
<prefix>.log.html), matching the CLI.Changed
<prefix>.psms.tsv→<prefix>.tsv; the crash-recoveryintermediate file renamed the same way (
<prefix>.intermediate.tsv).ms2rescore-report) reconstructsbefore/after rescoring comparisons from the main PSM list's provenance data and current state,
rather than relying on separately persisted result tables.
(run, spectrum_id)instead of barespectrum_id, so multiple input files reusing the same native spectrum IDs no longer collide.run_col,instead of relying on
spectrum_idalone.max_psm_rank_output > 1now applies consistently across the main output, rescoring tables,and report: multiple ranked PSMs per spectrum, with q-values/PEPs computed per-row rather than
through full spectrum competition. Intended for surfacing ambiguous candidates (e.g. from
Mumble), not a statistically rigorous FDR-controlled count.
id_decoy_patternis set.ms2rescore.utils(public) renamed toms2rescore._utils(internal) and merged with the newrescoring integration layer -- no longer part of the public API surface. [BREAKING]
pyarrow>=14.Removed
mokapotdependency removed, along with thems2rescore.rescoring_enginesmodule. [BREAKING]rescoring_engineconfiguration option removed (mokapot-specific:fasta_file,write_weights,write_txt,protein_kwargs), replaced byrescoring(see Added). [BREAKING]fasta_fileconfiguration option and FASTA-based protein inference removed. [BREAKING]lower_score_is_betterconfiguration option removed (now auto-inferred). [BREAKING]write_rescoring_tablesconfiguration option removed -- rescoring tables are unconditionallywritten now. [BREAKING]
log_level=debug-- the mainPSM list TSV already carries all rescoring features. [BREAKING]
Fixed
<prefix>.log.html), unlike CLI runs -- the GUI'slogging setup only ever attached a plain text-file handler.