Feat PyTorch calibrator and two-phase training pipeline#190
Feat PyTorch calibrator and two-phase training pipeline#190JemmaLDaniel wants to merge 38 commits into
Conversation
…ing training history recording fix: convert ProbabilityCalibrator OmegaConf objects into plain Python types
…s-compatibility' into feat-pytorch-calibrator
…formatting from TrainingHistory.plot()
| "matchms>=0.31.0", | ||
| "scikit-learn>=1.3.0", | ||
| "matplotlib>=3.7.0", | ||
| "torch>=2.10.0", |
There was a problem hiding this comment.
I would recommend keeping the same torch version ranges as InstaNovo.
Now Winnow is at "torch>=2.10.0" and InstaNovo at <2.5/<2.!/<2.9 (depending on the paltform and CUDA version), which makes it impossible to install Winnow and InstaNovo in the same environment or Google Colab.
| ``` | ||
|
|
||
| Koina model names, collision-energy / fragmentation inputs, and validity constraints | ||
| live in [`configs/koina.yaml`](configs/koina.yaml) (composed via `defaults: - koina` in |
There was a problem hiding this comment.
Running make docs retuns:
uv run mkdocs build --strict
│ ⚠ Warning from the Material for MkDocs team
│
│ MkDocs 2.0, the underlying framework of Material for MkDocs,
│ will introduce backward-incompatible changes, including:
│
│ × All plugins will stop working – the plugin system has been removed
│ × All theme overrides will break – the theming system has been rewritten
│ × No migration path exists – existing projects cannot be upgraded
│ × Closed contribution model – community members can't report bugs
│ × Currently unlicensed – unsuitable for production use
│
│ Our full analysis:
│
│ https://squidfunk.github.io/mkdocs-material/blog/2026/02/18/mkdocs-2.0/
INFO - Cleaning site directory
INFO - Building documentation to directory: /home/j-vangoey/code/winnow/docs_public
WARNING - Doc file 'configuration.md' contains a link 'configs/koina.yaml', but the target is not found among documentation files.
Aborted with 1 warnings in strict mode!
make: *** [Makefile:122: docs] Error 1
MkDocs resolves relative links only within the documentation source tree, not arbitrary repo files. Since configs/koina.yaml is not present as a docs page/file under the configured docs directory, MkDocs reports a broken link. Because --strict is enabled, that warning aborts the build.
The Material/MkDocs 2.0 message is just an upstream warning banner, not the build-breaking error. But something we propably will have to look into.
Likely fix is to use a full GitHub URL.
There was a problem hiding this comment.
Fixed with GitHub URL
| "dropped" if not self.learn_from_missing else "imputed as zero" | ||
| ) | ||
| warnings.warn( | ||
| f"Skipping experiment '{exp_name}':\n{e}\n" |
There was a problem hiding this comment.
When the input has no experiment_name and the global iRT regressor is skipped (for example because there are fewer than min_train_points or fewer than two unique peptides), _collect_training_data records "__global__" in _skipped_experiments, but this branch only applies skips when an experiment_name column exists. compute() then keeps those rows as valid and _apply_regressors() indexes self.irt_predictors["__global__"], raising KeyError instead of imputing or dropping the spectra as the warning says.
To reproduce: test_compute_imputes_skipped_global_regressor
There was a problem hiding this comment.
This is fixed. When the global RT -> iRT regressor is skipped and there is no experiment_name column, _mark_missing_spectra now marks all spectra as missing if __global__ is in _skipped_experiments, so compute() imputes them (or drops them when learn_from_missing=False) instead of reaching _apply_regressors() and raising a KeyError. The test now passes.
| if spectrum_path.is_dir(): | ||
| return _compute_features_directory( |
There was a problem hiding this comment.
With data_loader=winnow, the valid input is a saved Winnow dataset directory containing metadata.csv and optionally predictions.pkl . This branch intercepts every directory and sends it to _discover_experiment_files, which only accepts .parquet , .ipc, or .mgf files, so predict/compute-features now fail before WinnowDatasetLoader.load() can read a previously saved Winnow dataset directory.
To reproduce, see: test_compute_features_loads_saved_winnow_dataset_directory
There was a problem hiding this comment.
Fixed, _compute_features_batched_metadata now detects saved Winnow dataset directories (those containing metadata.csv) and passes them straight to _compute_features_single_file
|
|
||
|
|
||
| def notebook_safe_rich_handler(**kwargs: object) -> RichHandler: | ||
| """Return a Rich log handler that uses ANSI colours, not OSC-8 hyperlinks. |
There was a problem hiding this comment.
We might borrow this for InstaNovo as well!
There was a problem hiding this comment.
Thanks! Something I did for the PAASTA practical
| """ | ||
| irt_regressor_output_path = cfg.get("irt_regressor_output_path") | ||
| if irt_regressor_output_path: | ||
| from winnow.calibration.calibration_features import RetentionTimeFeature |
There was a problem hiding this comment.
A single calibrator/RetentionTimeFeature instance is reused per-file in directory mode, so _skipped_experiments accumulates. If file A skips an experiment and a later file B reuses that experiment_name, B's valid iRT features are silently marked is_missing_irt_error=True and imputed as zero (or dropped) → wrong calibrated scores, silently.
There was a problem hiding this comment.
Should be fixed with a _skipped_experiments reset in prepare() now, and I've added a test to catch it in future: test_skipped_experiments_reset_between_prepare_calls
| if should_stop: | ||
| break | ||
|
|
||
| history.epochs_trained = epoch + 1 # noqa: F821 — loop always runs |
There was a problem hiding this comment.
history.epochs_trained = epoch + 1 references the loop variable after the for loop. With max_epochs=0 (a valid, unvalidated int) the loop never runs, epoch is unbound and the result is an unhandled `NameError instead of a clear config error.
There was a problem hiding this comment.
Added a catch and a test test_max_epochs_must_be_positive
| ) | ||
|
|
||
| @torch.no_grad() | ||
| def predict(self, dataset: CalibrationDataset) -> None: |
There was a problem hiding this comment.
Training uses batched DataLoaders but prediction materializes the entire feature tensor + all activations at once. On millions of PSMs this OOMs VRAM (CUDA OOM) or exhausts host RAM.
| ) -> ProbabilityCalibrator: | ||
| """Load a calibrator from a local directory or HuggingFace Hub repo. | ||
|
|
||
| Expects the directory to contain ``model.safetensors`` and |
There was a problem hiding this comment.
load() no longer detects legacy pickle (calibrator.pkl) checkpoints. Pointing at a pre-PyTorch checkpoint dir now yields a bare FileNotFoundError. Raise a proper error that the old format is no longer supported.
There was a problem hiding this comment.
Added an error to handle this and a test test_load_legacy_pickle_checkpoint_raises
|
|
||
|
|
||
| @app.command( | ||
| name="diagnose-calibration", |
There was a problem hiding this comment.
diagnose_calibration loads via a single data_loader.load(**dataset_params) while predict streams directories one-at-a-time via _compute_features_batched_metadata. The same directory dataset that works under predict either errors or loads everything into RAM under diagnose-calibration.
| n_val = max(1, int(n * validation_fraction)) | ||
| n_train = n - n_val | ||
|
|
||
| generator = torch.Generator().manual_seed(cfg.get("calibrator", {}).get("seed", 42)) |
There was a problem hiding this comment.
_maybe_split_validation uses torch.Generator().manual_seed(seed) while _maybe_split_calibration_dataset uses np.random.default_rng(seed). Same seed will result in different orderings, so single-phase vs two-phase training produce different train/val partitions and can't be reproduced across modes. Factor into one shared index-generating helper.
There was a problem hiding this comment.
Refactored, the two modes should now agree for the same seed and dataset size
| import numpy as np | ||
| from winnow.datasets.feature_dataset import FeatureDataset | ||
|
|
||
| features, labels = calibrator._extract_feature_matrix(train_data, labelled=True) |
There was a problem hiding this comment.
_fit_from_calibration_datasets duplicates the feature-extraction + FeatureDataset wrapping from fit(), reaching into the private _extract_feature_matrix. If that return contract changes, fit() picks it up but the CLI copy silently doesn't.
Summary
Replaces the scikit-learn
MLPClassifiercalibrator with a PyTorchCalibratorNetwork, saved asmodel.safetensors+config.json. Adds a scalable two-phase train path (compute features to Parquet, thenfit_from_features) and revises compute-features / predict / diagnose-calibration CLI orchestration to match therevisionsbranch pipeline.What changed
Calibrator
hidden_dims, dropout, Adam training, GPU when availablen_iter_no_change+tol; optional validation subsampling (val_early_stopping_max_psms)TrainingHistory(epoch metrics, JSON export, plot)fit()/fit_from_features();predict()expects pre-computed features (no internalcompute_features)get_config()); Koina CE/frag runtime keys stripped on savekoina.input_constants/input_columnsonto loaded modelsData & training
FeatureDataset+from_parquet()(file or directory, optionalfeature_columns)winnow train: single-phase (features on full set → split → fit) or two-phase (features_path/val_features_path)resolve_data_path()for local paths and HuggingFace Hub models/datasetsCLI & config
compute-features/predict(per-experiment directories, shared filtering)configs/koina.yamlextracted; slimpredict.yaml/diagnose_calibration.yaml(no full train calibrator tree included where unnecessary)irt_calibrationpredict overrides; hybrid iRT (skip/warn on bad pools instead of raising to keep training runs going, keep Fix per experiment irt regression #188 loaded-regressor pinning)Makefiletargets usen_iter_no_changeTest plan
make tests/ pre-commitmake train-sampleandmake predict-samplefit_from_features, empty-dataset guardsfeature_columnstest_koina_intensity_config,test_paths(HF path resolution)