drevalpy v2 - #460
Draft
nictru wants to merge 444 commits into
Draft
Conversation
load_tissue_features and load_cell_line_id_features each forwarded their two arguments to a datasets.feature_tables function and added nothing else. No caller ever reached them: the module's own dispatch calls load_tissues_from_csv and load_cl_ids_from_csv directly, so the wrappers only ever appeared in the facade that re-exported them.
The six thin wrapper functions in view_resolution.py are deleted. ModelConfig gains cell_line_views(), drug_views(), cell_line_entity_id_only(), and drug_entity_id_only(); ResolvedModelConfig gains cell_line_views() and drug_views() that forward to the template with resolved=self. All callers updated; no behaviour change.
No circular dependency exists since hyperparameter_space.py only imports from the standard library.
These methods are the active checkpoint persistence protocol, not legacy bridges.
Every subclass duplicated the same `if batch.response is None` check. Now base `fit()` validates once and delegates to abstract `_fit()`.
Structural invariants (response/pair-index lengths match n_pairs) are now enforced at construction time. This removes the scattered validate_matrix_fit calls and deletes the _matrix_fit module entirely.
The batch now guarantees response is always float64 when present, removing redundant np.asarray(..., dtype=np.float64) casts from every predictor _fit method and from batch construction helpers.
_fit and predict are now @AbstractMethod, matching how MatrixPredictor and FeatureFreePredictor already require subclass implementations. Also updates the doc example to use _fit.
batch.response is already guaranteed float64 by ModelInputBatch.
One-hot values are exact 0/1 integers; floating-point tolerance is unnecessary.
…ictor. Backward compatibility for old single-algorithm payloads is no longer needed; set_state now only accepts the modern per-drug 'algorithms' mapping format.
Each of the 8 literature predictors (SRMF, SparseGO, Precily, PharmaFormer, MOLIR, SuperFELTR, DIPK, DrugGNN) now subclasses BlockPredictor and implements _fit/predict by reading feature blocks from the batch, eliminating the FeatureDataset/LiteratureTrainingMixin bridge layer entirely. Deleted bridge infrastructure: - SingleDrugBlockPredictor, FeatureDatasetBlockPredictor - _algorithm_lifecycle, _block_inputs, _feature_dataset_from_batch - _training_helpers (LiteratureTrainingMixin) - Per-predictor algorithm.py and state.py bridge files
Replace per-predictor custom Dataset classes with a single `make_tensor_loader` helper that converts numpy arrays to a TensorDataset-backed DataLoader. MOLIR and SuperFELTR now pre-expand entity-level omics matrices to pair-level before training, removing all FeatureDataset and RegressionDataset dependencies.
…kage Relocate Predictor, BlockPredictor, MatrixPredictor, and FeatureFreePredictor into drevalpy/components/predictors/abstract/ for clearer separation of the class hierarchy from concrete implementations. Re-exports from __init__.py preserve convenience imports.
lightgbm, xgboost, requests, mygene, and obonet are declared core dependencies — remove try/except ImportError wrappers and import them at module level. Installed the missing packages in the dev venv.
Introduce IndexedPairDataset that stores entity-level feature matrices compactly and performs the entity-to-pair lookup per sample in __getitem__. This eliminates O(n_pairs * d) memory allocations that caused OOM on large drug-response matrices — memory now scales with O(n_entities * d + n_pairs) instead. All 6 torch-based predictors (NeuralNetwork, MOLIR, SuperFELTR, PharmaFormer, Precily, SparseGO) updated to use the new make_pair_loader with entity matrices + pair indices.
Eliminates the module-level _CONSTRUCTED_CACHE dict and _canonical_config_key helper in favour of @functools.cache on _generate_model_class, reducing boilerplate while preserving class-identity stability guarantees. Co-authored-by: Cursor <cursoragent@cursor.com>
The `resolved` parameter was always provided in practice; remove the unused `config` fallback kwarg and make `resolved` non-optional. This tightens the type signatures of the .config and .resolved properties, eliminating None-checks at call sites. Co-authored-by: Cursor <cursoragent@cursor.com>
Hoist wandb, data_loading, and _model_persistence imports to module level since they have no circular dependency on drp_model. The tuning imports must remain deferred due to the DRPModel ↔ tuning cycle. Co-authored-by: Cursor <cursoragent@cursor.com>
pytorch-lightning is a required dependency so the deferred import was unnecessary. Co-authored-by: Cursor <cursoragent@cursor.com>
Fixes xdoctest CI failure caused by stale lock file. Co-authored-by: Cursor <cursoragent@cursor.com>
When a predictor's optional dependency (e.g. lightgbm) cannot be imported, the model_validator now gracefully skips semantic checks instead of propagating the ImportError. The config remains structurally valid; full validation occurs at runtime when the predictor is actually used. Fixes test_subprocess_blocks_optional_deps_for_simple_models in CI. Co-authored-by: Cursor <cursoragent@cursor.com>
- Add None-checks for batch.response and drug_pair_idx before use - Narrow ModelConfig | ResolvedModelConfig unions with isinstance assertions - Add missing pair_idx argument to train_superfeltr_model calls - Annotate frozenset variables to satisfy var-annotated checks - Remove unused type: ignore comments - Use cast() for metadata dict values in SparseGO predictor - Add proper type annotations for registry variable in register_builtins - Suppress read-only property assignment errors in intentional test code Co-authored-by: Cursor <cursoragent@cursor.com>
- Defer data_loading imports in DRPModel to avoid circular import in subprocess isolation tests (also fixes monkeypatch ineffectiveness in test_load_drug_features_uses_featurizer_loader_when_configured) - Update ExternalPredictor stub to implement _fit instead of fit - Move test_feature_free.py to match renamed source at abstract/ - Call _set_xgboost_thread_defaults() in XGBoostPredictor.set_state - Use squeeze(dim=-1) in FeedForwardNetwork.forward to prevent 0D output tensors when batch_size=1 Co-authored-by: Cursor <cursoragent@cursor.com>
…ircular import The top-level import triggered featurizers/__init__ → cell_line/__init__ → bionic → predictors/__init__ → abstract.block → model_input_batch (cycle). Moving to a deferred import in feature_matrix_for() breaks this chain. Co-authored-by: Cursor <cursoragent@cursor.com>
- SparseGO: replace cast() with actual list()/dict() conversions so typeguard sees correct concrete types at runtime - test_metadata: accept TypeCheckError alongside TypeError for intentional bad-type tests under typeguard instrumentation - test_registration_metadata: same Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Gene-list resolution searched the download cache and a repo-local data/meta path, neither of which exists in an installed environment, so landmark featurizers crashed in the container pipeline. The lists now live next to the featurizers and are resolved only from there, making gene selection independent of cache and download state.
Every pipeline worker on AWS Batch started with a cold HuggingFace cache, because the Nextflow modules point XDG_CACHE_HOME at the per-task work dir. Hundreds of concurrent tasks therefore pulled the 342 MB checkpoint anonymously through a single NAT gateway, HF replied with 429s, and transformers reported it as a misleading "Can't load the model" OSError. The mirrored snapshot loads bit-identical weights and is now cached per process rather than reloaded for each of the 17 fits in a tuning run. Also drop the hardcoded "orakl" AWS profile, which exists in no environment and made every artifact download raise ProfileNotFound, and repoint the dataset registry at the bucket that actually holds the .h5mu files.
tests/conftest.py acquired its fixtures by adding scripts/ to sys.path and importing download_dataset from it, but scripts/ is not in the repository and data/ is gitignored. The ImportError was swallowed by a bare except that returned False, so a clean checkout produced 1205 FileNotFoundError setup errors instead of one clear failure. Every test was affected, including test_cache_dir.py, which has no data dependency at all: a session-scoped autouse fixture reads the TOY CSVs. The .pre-commit-config.yaml pytest hook runs the full suite, so committing was equally broken on a fresh clone. Nothing in the package ever read those CSVs; they were written and read only by conftest. Fixtures now come from tests/synthetic/, which assembles a 24x8 raw-omics MuData in memory and lets the real featurizers derive fingerprints and drug graphs from real SMILES, so the fixture cannot drift from production. conftest.py drops from 485 to 106 lines and no longer mutates DREVALPY_CACHE_DIR at import time, which is what masked the breakage locally. Featurizers needing pretrained weights are marked network and deselected, as their artifact bucket is not readable without credentials. The suite now passes from a clean export with no data/ and no scripts/. Three library defects surfaced while removing the scaffolding: - Omics reads passed the public view name straight to mdata.mod, so MOLIR, SuperFELTR and the MultiView presets asked for copy_number_variation_gistic while every published .h5mu stores copy_number_variation, and failed for all users. Reads now resolve through OMICS_ACCESSORS, preferring the name as written so both dataset generations work through one path. - PredictorBase.fit filters NaN pairs across the whole batch, but subset_pairs rejected multi-drug masks whenever early stopping was active, which broke nine models. Set membership generalizes it; the single-drug case is the one-element instance of the same rule. - MolGNet multiplied a list by data.num_nodes, which is Optional[int], and ChemBERTa called a tokenizer that its own type allowed to be None. Fixing the ten hasattr sites ty reported also collapsed five copies of the same sparse-densify block into one to_dense helper. ty now reports zero diagnostics, down from 14. Finally, tests mirror the source tree one file per public module, documented in AGENTS.md. Three test directories mirrored packages that no longer exist after the registry consolidation in c76a78b. Uncovered public modules drop from 104 to 76 and are tracked by the existing warn-only mirror policy test rather than by stub files.
The quickstart could not be followed. It invoked flags removed in v2 (--run_id, --dataset_name, --test_mode, --path_out, --baselines), imported load_mudataset and mu_experiment, which are absent from the package, and told readers to install with Poetry although the project builds with uv and hatchling and has no [tool.poetry] section. Every example loaded TOYv1 or TOYv2, which are legacy v1 datasets that the registry does not contain, so they would fail even spelled correctly. Examples now use GDSC1 and the real CLI and Python surfaces, and the dataset tables list exactly the seven registered names. cli/pipeline_commands.rst is deleted rather than rewritten: all 17 stepwise commands it documented were removed in v2, so there was nothing left to correct, and the surviving CLI is generated from the Typer app into cli/_generated_reference.rst. A hand-maintained duplicate is how these pages drifted in the first place. The nextflow blocks in the README kept their parameters, which are real pipeline options rather than drevalpy flags, and now say so. dreval_colab_demo.ipynb is removed; it targeted the pre-v2 API and was one of only two sources of unresolved-import diagnostics. make_gene_lists.ipynb becomes _make_gene_lists.py so the provenance of the shipped gene lists stays readable and lintable. The leading underscore keeps it out of component discovery, which scans package directories for public modules. Its per-omic dataset lists lose the TOY entries rather than substituting one name, since each list enumerates the datasets carrying that omic; CCLE is consequently the only proteomics source. Also correct a documented seven-fold CV default that is five, a drevalpy-report entry point that is not declared, a --wandb-project flag that exists only in the Python tuning API, and a nox session with no noxfile.
The file is Finder metadata with no bearing on the project, but it was committed and unignored, so it reappeared in git status on every macOS checkout and kept getting swept into unrelated commits.
The test suite had no way to notice when a module shipped untested: 76 of 148 public modules had no mirrored test file, and whole user-facing areas were unguarded - the CLI is the primary entry point and had zero tests, `types/results/` was never constructed by any test, and `visualization/` sat at ~21%. Reported coverage also flattered us, because bare `--cov` measured `tests/` itself and silently omitted package files that nothing imported. Scope coverage to the package, mirror the remaining modules, and enforce both in the hook so the gap cannot silently reopen: - `[tool.coverage.run] source = ["drevalpy"]` so never-imported files count as 0% instead of vanishing from the report. - Two-level gate: global `fail_under = 88` plus a per-file floor of 60% via `tools/coverage_gate.py`, whose exemption table records each below-floor module with the reason it cannot comply yet. The gate prints redundant entries so the debt list visibly shrinks. - Wire `pytest --cov-report=json` and the gate into pre-commit and CI, so `--no-verify` cannot bypass the floor. - Close the mirror backlog and flip `test_module_mirror_policy` from warning to failing, accepting the underscore-stripped names AGENTS.md already prescribed. One module is exempt: the omitted maintenance script `_make_gene_lists.py`. Coverage 67% -> 89.5%; 1242 -> 3044 tests in ~57s. Tests pin observed behaviour rather than intended behaviour where the two diverge, so several latent bugs are now documented by passing tests instead of hidden: `SuperFELTRegressor` keeps its encoders in a plain tuple and so breaks on GPU/MPS, `drevalpy/__init__.py` shadows its own submodules and defeats dotted-path monkeypatching, and the pinned curve_curator fork makes `fit_type="MLE"` unreachable. These are left unfixed here to keep the change reviewable.
Closing the test gap turned up six defects that had no coverage to catch them. Each fix comes with the regression test that would have caught it. SuperFELTRegressor kept its encoders in a plain tuple, so Lightning's device placement never reached them: on a GPU/MPS host the regressor moved and the encoders stayed behind. Registering them in an `nn.ModuleList` fixes placement but would also start training pretrained weights and let `trainer.fit`'s `train()` call reactivate their BatchNorm statistics, so the optimizer is narrowed to the regression head, the encoders are frozen, and `train()` is overridden to keep them in eval. The CPU-pinning test fixture that worked around this is gone, which is the real proof. `fit_type="MLE"` was reachable from both `curate()` and `--fit-type` but always died inside the pinned curve_curator fork with `_Model.fit_mle() got an unexpected keyword argument 'weights'`. It is now rejected up front with a message naming that incompatibility instead of failing after preprocessing; the private plumbing still forwards the value, so re-enabling is a one-line change. `single()` handed `split_masks.metadata` to `RunResult` by reference, and `run()` reuses those dicts across models and folds, so mutating a result corrupted its own inputs. It is copied now. `drevalpy/__init__.py` re-exported `run`, `single`, `load`, `randomization` and `robustness` as functions that shadowed the same-named submodules, which broke `import drevalpy.run` and made dotted-path monkeypatching fail in a way that reads as a test bug. The five modules are now private, so the public names are unambiguously the functions. Dropping the submodules also unmasked `drevalpy.experiment` re-documenting `RunResult`, which is owned by `drevalpy.types.results`; it stays importable but leaves `__all__`, keeping the `-W` docs build clean. Also: deleted unreachable guards in `model_input_build.py` that a prior validation call always pre-empted, and made `config_lock()` reentrant via `is_singleton=True` so nesting two blocks no longer deadlocks to the 10s timeout (filelock pinned to >=3.15.2, where singleton init settled).
Replaying 6337 completed Optuna trials from a CTRPv2 benchmark sweep turned up several ranges whose upper halves bought no accuracy: within a predictor the expensive settings scored no better than the cheap ones, so part of every trial budget was being spent on runtime instead of on tuning. Each range is cut back to the region the trials actually rewarded. - chemberta: max_length drops 512 and defaults to 256. Attention cost is quadratic in sequence length and no benchmark SMILES needs 512 tokens. - fingerprints: n_bits drops 4096, which doubled the design matrix for indistinguishable scores. - xgboost: max_depth 12 -> 8, n_estimators 500 -> 300. lightgbm: n_estimators 500 -> 300. - randomForest: max_samples 0.9 -> 0.5. gradientBoosting: max_depth 30 -> 12. - adaboost: n_estimators 200 -> 100, and min_samples_split/min_samples_leaf become non-tunable, having shown neither runtime nor accuracy signal. ElasticNetPredictor declared max_iter, tol and selection in non_tunable_hyperparameters, but _make_estimator never forwarded them, so all three were silently discarded and sklearn's defaults applied instead. The forwarding fix ships here rather than separately because it is what makes the convergence settings reachable at all; the two are not independently meaningful. Ridge stays out of the selection= path, not being coordinate descent. Forwarding then exposed a reproducibility bug that the dropped arguments had been masking: selection="random" with random_state=None takes its coordinate order from unseeded global randomness, so two fits on identical data disagree, which is enough to fail the facade/resolved-config parity test. Both coordinate-descent predictors are seeded with random_state=0 now. Keeping random selection instead of reverting to cyclic is deliberate - sklearn notes it "often leads to significantly faster convergence especially when tol is higher than 1e-4", and tol=1e-2 over features as correlated as ~867 landmark genes plus 2048 fingerprint bits is exactly that regime. LassoPredictor carried the same missing seed independently of the ElasticNet change, so it is fixed alongside; its max_iter also leaves the tunable space, since the binding constraint on convergence was feature scaling rather than the iteration budget.
`response_transformation` never worked. A default of `None` was the only reason nobody noticed, because every way of enabling it either corrupted results or crashed: - Training read raw responses straight from `response_matrix` while predictions were inverse-transformed regardless. With `standard` that multiplied predictions by the training std and added the mean, so the transform degraded output instead of round-tripping. - Nothing in the HPO path ever called `fit`. `single()` passed the unfitted prototype into `hpam_tune` and fitted only its own clone afterwards, so `inverse_transform` raised `NotFittedError` in every Optuna trial. - No CLI flag existed, leaving the pipeline's `response_transformation` parameter with nothing to reach. The contract is symmetric now: fit on the training scope only, transform the training target, inverse-transform predictions, and score against the untouched response matrix so metrics stay in original response units. `fit_response_transformation` is the one place a response transformer is ever fitted, and it clones the prototype so callers can reuse it across folds. The fitted instance reaches training through a keyword argument rather than by rewriting `Dataset.response_matrix`, because ground truth reads that same property -- transforming it in place would have transformed the targets too and silently cancelled the inverse out. Four of the six `_extract_response_pairs` call sites take the transform: both `_ComponentStack` training paths, its early-stopping supervision (training-time targets, so they must share the space), and `DRPModel.train`. The two evaluation sites stay raw. That split is invisible in the metrics when wrong, so tests pin each site. The CLI defaults to `standard`. Affine target scaling is a no-op for squared-error trees and KNN, which the equivalence test asserts, so most of the benchmark is unaffected; gradient-based predictors optimize better on unit-variance targets, and the regularized linear models gain an `alpha` range whose meaning no longer depends on the response scale. Note this shifts the reported baseline for those two families. The docs pages for both entry points described the old behaviour -- the CLI page claimed `single` took the same options as `run`, and the Python page documented the argument as accepting strings rather than a `TransformerMixin` -- so both are corrected here.
Generating a report for 97 models was OOM-killed at 36 GB and again at 60 GB. comparison_scatter retained a full point cloud for every model pair: at 96 models that is 4,560 pairs x 231,080 two-key dicts at 240 B each, about 253 GB, so no container size could have worked. regression_scatter independently retained 5.33 GB of point dicts across 96 models, and wrote all 22.2M of them to multiquc_data. Replace both with bounded aggregations. comparison_scatter now stores a float32 models-by-groups correlation matrix (0.24 MB) and plots per-drug and per-cell-line correlation behind two model dropdowns, restoring the semantics the predecessor implementation documented. regression_scatter becomes a hexbin density image, which is what MultiQC already flattened these plots to above plots_flat_numseries. Retained state is now flat in the number of predictions rather than quadratic in the number of models. Stop loading data no plot reads: HPO trial predictions (2.37 GB) are skipped through with_trials=False, drug_ids and cell_line_ids are interned out of fixed-width <U40/<U9 arrays (4.4 GB to 0.4 GB), and the .h5mu is no longer read at all since every compute() ignores it. Loading drops 10.3x. Fixing the memory alone still produced no report. leaderboard asked for a "Pearson: normalized" metric key that normalize() never emits, so the PCC column was entirely NaN and set_xlim rejected the axis. It passed CI only because the synthetic fixture injected that key unconditionally. Resolve metric names through a shared helper that accepts either spelling, derive axis bounds from finite values while admitting negatives, and warn and skip rather than crash where a metric is absent. heatmap, violin and critical_difference shared the same assumption. Add memory and progress logging, emitted before each plot allocates, so the next failure of this kind names its culprit instead of exiting 137 in silence. Build figures off matplotlib.figure.Figure so pyplot's global registry no longer retains them. Peak RSS on the real 97-model results is 2.62 GB in 4m24s.
A plugin whose import raised was swallowed into a logger.warning, so every
component it would have registered simply vanished, and the absence resurfaced
much later as an "unknown predictor" error far from its cause. Load outcomes are
now recorded and readable through get_failed_plugins() and get_loaded_plugins(),
and DREVALPY_STRICT_PLUGINS=1 re-raises so a plugin's own CI fails on its own
bugs. The default stays non-fatal: one broken third-party package should not
brick the CLI for everyone else.
Plugin authors previously had to reach into five deep module paths that were free
to move under them. drevalpy.plugin now re-exports the 45 symbols an extension
actually needs and drevalpy/py.typed ships the type information that goes with
them, so the internal layout stays free to change while the aliases stay a
promise. drevalpy.testing ships the synthetic dataset and batch builders,
check_plugin and the conformance checks that until now every plugin repo
hand-rolled. docs/python/extensions.rst is rewritten against the real API, and
its examples live as executable modules under docs/examples/ that the docs build
imports and registers, so they cannot rot unnoticed. A new `drevalpy list`
sub-app exposes all five registries plus plugin load status.
Two import-time side effects are gone. critical_difference.py called
matplotlib.use("agg") at module scope, and because importing drevalpy imports
every builtin visualization, merely importing the library disabled inline
plotting in any notebook; matplotlib already falls back to agg without a
display, so the line bought nothing. Separately, the default editable install
put the bare project root on sys.path, so a consumer's `import tests.conftest`
resolved to drevalpy's own tests -- hatch's dev-mode-exact maps only the
drevalpy package, which is why `editables` had to become a real project
dependency rather than a dev-group one.
Breaking changes to the registration API:
- Registration now rejects classes with unimplemented abstract methods rather
than accepting them and failing at first use.
- A class-body `contract` (or `cell_line_contract` / `drug_contract`) is a valid
declaration where it previously raised TypeError. The decorator argument still
wins when both are given.
- Registering an already-registered splitter mode raises instead of silently
overwriting it, with override=True for the cases where replacement is meant;
visualization registration, which already raised, gained the same escape
hatch.
- visualization.table() returns a DataFrame instead of a str.
- metadata() was added to the splitter and visualization registries so all five
can be introspected the same way.
Component validation also moved ahead of first use, so a malformed component is
rejected at registration. fail_under moves 88 -> 89 now that measured coverage
sits at 90.1%.
The pre-commit hook ran the whole suite with coverage: 98.6s warm, 281.5s cold. Measuring it showed the cost was not the 3830 tests - 3397 of them contribute ~0.0s between them - but the ~30 that spawn a fresh interpreter, each paying `import drevalpy`. That import cost 3.59s, because `drevalpy/registry/__init__.py` registers builtins eagerly and every registered predictor pulled its training stack in at module scope. Move the heavy imports into the functions that need them (16 libraries, including pytorch_lightning, torch_geometric, torch, sklearn and mudata), keeping registration eager so a fresh process still discovers all 27 predictors and 27 featurizers. `import drevalpy` is now 0.21s and the CLI starts in 0.24s, which every user pays on every invocation, not just tests. tests/test_import_cost_policy.py locks this in; two shapes it guards do not have the "move the import into the method" remedy, so they are documented: a forbidden base class must move module, and a module-scope side effect must stay eager - deferring the xgboost thread defaults segfaults the suite once torch's OpenMP is loaded first. Then split the suite. `uv run pytest` now defaults to the fast tier via addopts and takes 15s for 98% of tests; the 65 tests that spawn interpreters, fit curves or train models carry `slow` and run in CI. A test earns the mark by costing >=0.2s measured as wall time saved when its whole fixture-sharing group is deselected - not by summed per-test durations, which now mislead because caching hoisted the cost into shared setup. Coverage and the per-module gate move to CI on the full suite, where the floors are meaningful; a green hook no longer says anything about coverage, and AGENTS.md says so. Share the fixtures that were rebuilt per test: one wheel build, one fitted curve set, one report. No tests were deleted; the suite grew by 79 and coverage went 90% to 91.13%. Three latent bugs surfaced, each invisible only because serial collection order hid it: - tests/visualization/test_base.py faked IPython without `version_info`, which matplotlib probes once per process when the first canvas is created. It passed only when an earlier test had already tripped the probe. - test_spec.py, test_extensions.py, test_validation.py and test_block_specs.py leaked registrations into the global registries. The last three called register_builtin_components() as teardown, which only adds and never evicts. test_extensions.py was saved by two unrelated tests' finally blocks. pytest-xdist is deliberately not adopted: with the import cost gone it gains at most ~11% on the CI suite and is slower on the fast tier. It earned its keep as a diagnostic - it is what shook the order dependencies out.
A `pca[gene_expression]:identity:lightgbm` run in the container died with
`OSError: libgomp.so.1: cannot open shared object file`. LightGBM's wheel is
the only one in the dependency set that links the system OpenMP by bare
soname: `lib_lightgbm.so` carries `NEEDED libgomp.so.1` with no RPATH and no
vendored copy, while the xgboost and scikit-learn wheels ship an
auditwheel-renamed `libgomp-e985bcbb.so.1.0.0` and find it through their own
RPATH. The runtime stage is `python:3.13-slim-bookworm`, which has no
`libgomp1`, and only `/opt/venv` is copied out of the full-bookworm builder,
so the builder's copy never reaches the image.
It had never been satisfied properly, only by accident. torch bundles
`torch/lib/libgomp.so.1` with the *unrenamed* soname and loads it RTLD_GLOBAL
through `libtorch_global_deps.so`, so once torch was imported the loader found
`libgomp.so.1` already in the link map and never touched the filesystem. Every
predictor used to pull its training stack in at module scope, so `import
drevalpy` imported torch and LightGBM worked as a side effect. Deferring those
imports removed the accident, and a lightgbm-only run - which never imports
torch - was the first to notice. An `ldd` sweep over every `.so` in the image
confirms `lib_lightgbm.so` is the only file left with an unresolved
`libgomp.so.1`, so `libgomp1` in the runtime stage is the whole fix; reverting
the deferral is not, since tests/test_import_cost_policy.py exists to keep
torch out of `sys.modules`.
The build now imports each native stack in a separate process. Separate
processes are the point: a single combined import passes on an image missing
libgomp1, because importing torch first resolves LightGBM's dependency for it,
which is exactly how this stayed hidden. A missing system library fails
`docker build` instead of surfacing 200 lines into a Nextflow log.
That log is the second half of this commit. All 16 Optuna trials failed with
this OSError, and the run still reported `Best hyperparameters: {...}` before
dying later in `model.train()`, because a raising trial was logged and turned
into NaN and `_resolve_best_params` then fell back to defaults. A trial that
runs and scores non-finite is an inconclusive tuning result and defaults are
the right answer there; a trial that raises is a fault, and if every one of
them raised there is nothing to fall back from. `hpam_tune` now raises
`HPOTrialsFailedError` in that case, chaining the first exception, so the
OSError heads the traceback instead of being 200 lines of scrollback away from
the line that reported success. A study where only some trials raised still
tunes, and warns with the counts. Trials keep returning NaN into the study, so
a search space with a few invalid corners behaves as before.
`_optuna_objective` in hpo_runtime.py has the same swallow but is left alone:
`build_optuna_objective` has no shipped caller, and a second copy of this
logic would be worse than the duplication is now.
Verified in the rebuilt image: lightgbm imports and fits with torch absent
from `sys.modules`, `import drevalpy` still leaves torch unimported, and the
failing model spec runs through `single()` with HPO to tuned hyperparameters
rather than defaults. Hiding `libgomp.so.1` again reproduces the original
fault as `HPOTrialsFailedError` with the `OSError` as `__cause__`, no
`Best hyperparameters` line and exit 1.
The packaged registry gains the seven orakl_v2 screens plus TOYv1, which broke two tests that asserted against a literal seven-name list and a literal count. Derive both from available_datasets.json instead, so the next dataset addition does not need a test edit, and assert the invariant that actually matters: builtin_datasets and builtin_sources mirror the packaged JSON exactly. The original seven stay behind as an explicit presence guard so an accidental deletion still fails.
The refit screens ship every fitted curve, and they store pEC50 as the response matrix. A pEC50 exists for every curve that converged, so `~isnan(response_matrix)` counted all 384 pairs of TOYv1 as usable observations - including the 163 the fit scores as noise. Every splitter was training and evaluating on them. `curve_quality_mask` exposes each CurveCurator quality metric the datasets carry as a keyword option, and the four built-in splitters now blank the failing pairs before splitting. Called bare it applies `relevance_score >= -log10(0.05)` and `abs(fold_change) >= 0.45`, the alpha and fc_lim this repo already passes to CurveCurator; on TOYv1 that reproduces CurveCurator's own `regulation != 0` label with zero mismatches across all 384 pairs. Gating on relevance_score rather than p_value is the load-bearing choice: p_value is the raw, uncorrected F-test value, and with mtc_method = "sam" the multiple-testing control lives entirely in relevance_score. Filtering on p_value across the ~60k curves in a screen would correct for nothing. It stays available, off by default, along with every other metric, so a plugin can filter on anything the dataset holds. A table drives the checks so fifteen options do not mean fifteen branches, and the tests parametrize over it - a new rule cannot land without a boundary and an off-by-default case. Consequences worth flagging: - The layers are guaranteed by the file format, so there is no capability check and no silent fallback; a missing layer raises. That means the older generation of the screens, which spells the metrics in CamelCase and has no `regulation`, can no longer be loaded, and the registry now points the plain dataset names at the refit files. CCLE is dropped until its refit exists in the bucket, which also empties the proteomics gene list - the tuples are updated and the shipped CSVs left alone. - The refit reused the old file names, so a cache from an older drevalpy holds a file that parses but has no quality layers. `load` served it forever and it then failed deep inside a split, so a file that cannot be quality-filtered is now treated as stale and fetched again. - `MuDataLike` gains `response_layer_names` and `get_response_layer`. Breaking for anyone implementing the protocol by hand; documented.
Two independent defects in the curve fitting, both silent. CurveCurator computes a standard error for every fitted parameter on every fit at every speed, from a Moore-Penrose pseudo-inverse of the Jacobian, and emits pEC50 Error alongside the slope, front and back errors. All four were dropped on the floor because _COLUMN_RENAME did not list them, so the pipeline shipped a million curves with no uncertainty estimate at all - the one number that says how much to trust a pEC50. They are now renamed and carried through as layers, pec50_error next to the X it quantifies. Normalization ran inside each parallel chunk, and curve_curator derives its factors from the median over the rows of whatever frame it is handed. With cores=8 a normalized dataset therefore got eight independent sets of factors, which made its output a function of the core count - reproducible only by accident. _normalize now computes the factors once per dose-range group before chunking and disables curve_curator's own normalization, so the split cannot be observed. Signal Quality needs care here: run_pipeline derives it from the raw control intensities that normalization overwrites, so it is computed before the overwrite, carried on the frame and restored after the fit. tests/curation/test_normalize.py compares two core counts byte for byte. Also names the fit speeds, defaults them through DEFAULT_FIT_SPEED, and rejects an unknown speed up front rather than letting curve_curator fail mid-fit.
The response matrix holds pEC50 - build_anndata pivots that column into X - but the leaderboard subtitled every plot "LN_IC50", so a reader comparing runs was told the wrong measure. Higher pEC50 is more potent while lower LN_IC50 is, so the label inverted the direction of the axis it described. ALLOWED_MEASURES went with it. Nothing in drevalpy imported it; its only reference was a test asserting the list was well-formed, which made it look like a validated contract while validating nothing. A measure name is checked where it is used - get_response_layer raises on a missing layer and names the ones that exist - so the list was a second, unenforced source of truth. Also lifts the CSV/Parquet read in the curate command into a helper and records in both docstrings why the .h5ad is the intermediate the curation pipeline wants: curate keys obs_names/var_names from the label columns it was handed, so a caller can fit on native identifiers and remap later without refitting.
The dependency list had drifted in both directions: entries nothing imports, and imports nothing declares. Seven declarations had zero import sites. `starlette` was the clearest case - it was added as a `>=0.49.1` floor three days after CVE-2025-62727 to push a *docs-only* transitive (sphinx-autobuild's live-reload server) past the advisory, but it landed in [project.dependencies], so every install carried an ASGI framework. Also gone: `mygene` and `obonet`, with no reference anywhere; `toml`, because both drevalpy and curve_curator read TOML with stdlib `tomllib`; `typing-extensions` and `importlib-resources`, both redundant at `>=3.11`. `flaky` moves to the dev group, where its only user (tests/test_evaluation.py) lives, and `pygments`/`types-toml` leave it - the latter stubbed the package above. `fsspec[full]` becomes `fsspec[s3,http]`. Only `s3://` (the default artifacts URI), `http(s)://` and `file://` (fsspec core) are reachable from shipped code, but `full` is 18 backends, so the lock carried dask, distributed, panel, paramiko, pygit2, dropbox, adlfs, the google-cloud and grpcio stacks, ocifs and smbprotocol to serve protocols nothing here can reach. Registering another protocol now means installing its backend, which fsspec reports by name. Three imports were undeclared and worked only by accident. `anndata` is the curation pipeline's return type and is imported at module scope in `curation/_anndata.py`; `joblib` is imported at module scope in `_persistence_io.py`, so model save/load would have broken had scikit-learn dropped it; `pyarrow` is the engine behind `cli/curate.py`'s `pd.read_parquet`, and reached us only via `fsspec[full]` - narrowing that extra without declaring it would have broken `drevalpy curate input.parquet`. All three are now explicit. `torch-scatter` was the most expensive dependency per line of use: a compiled extension with no wheels for current torch, needing the `extra-build-dependencies` escape hatch to build at all, for a single `scatter(..., reduce="add")` in MolGNet's hand-rolled message-passing base. torch_geometric has vendored an equivalent `scatter` since 2.3 and no longer requires torch-scatter, and the file already imported from `torch_geometric.utils`. The project now compiles nothing from source: a clean install with no pre-existing torch went from a build-isolation failure to 5.6s. Verified rather than assumed: embeddings from the real 300 MB MolGNet.pt, over five molecules spanning 3-15 atoms, are `np.array_equal` before and after with torch_scatter uninstalled. Network tests pass unchanged (7 passed, 1 skipped - the skip is a ragged-varm limitation, not auth), the full `-m "not network"` suite is 4125 passed / 9 skipped, molgnet still registers with `get_skipped_builtin_modules()` empty, and all 10 prek hooks pass. uv.lock drops 363 -> 302 packages.
`.repowise/`, `.vscode/` and `.mcp.json` are per-machine: an MCP server list, editor extension recommendations and a knowledge-graph cache. They carry no meaning for another checkout and were only showing up as noise in `git status`.
Repowise had wired Claude Code, VS Code and Cursor from one init, though Cursor is the only host installed here. Unwiring the other two removed .mcp.json, so its ignore entry named a file that no longer exists, while the .cursor/ config it did leave behind was unignored and embeds an absolute path to this checkout. .vscode/ stays: repowise rewrites it on every update regardless of the vscode_mcp opt-out.
The file had grown into a narrative of how the test suite got fast: exact timings, benchmark tables, and the war stories behind each guard. Those numbers go stale on the next commit and buried the rules that still apply, so collapse each story to the rule it implies, gather every command into one block at the top, and promote the import-cost guard out of a nested bullet.
Every featurizer indifferent to which entity side it serves was written twice, once under cell_line/ and once under drug/, differing only in a class name, a base import and a registry import: identity.py was 106 lines on each side with 20 of them differing, constant.py was 100% duplicated. The copies bought nothing, because side is stamped onto the class by the registry, not by the per-side base, which is empty. That duplication alone made those two the worst directories in the repo. A side-agnostic featurizer now lives once under featurizers/shared/, and register_for_sides derives, registers and namespace-injects one subclass per side. The dense-view base that had also been copied per side collapses into a single DenseViewFeaturizer, leaving each single-view featurizer with only its own transform step. The storage cluster moves off Featurizer into FeaturizerStorageMixin, which is what had made the class incohesive. The hyperparameter key grammar had the same problem in three copies, one of them justified in its own docstring as a way to break an import cycle between models/config and models/tuning. It now has a single home in a dependency-free leaf, so neither package reaches through the other. build_optuna_objective and a validate_merged_mapping shim had no callers at all and are gone. Separately: plotly no longer loads on import drevalpy, where it cost 0.12s to build validator tables for plots nobody had asked for yet, and it joins the forbidden-startup-import guard. The re-export barrels with the widest fan-in gained tests pinning their surface against the defining leaf modules rather than sibling barrels, so the assertions can actually fail. drevalpy.plugin exports the reuse machinery, so a plugin author writing a side-agnostic featurizer is not pushed straight back into hand-writing both copies. Breaking: ConcatFeaturizersCellLineFeaturizer and ConcatFeaturizersDrugFeaturizer are now CellLineConcatFeaturizer and DrugConcatFeaturizer, DenseViewCellLineFeaturizer moves to cell_line.base, and ViewDrugFeaturizer is no longer a base class. No compatibility shims. Registry names are untouched, so configs keep working.
Add an optional `executor` parameter to `curate()` and `fit_groups()` that accepts any `concurrent.futures.Executor` instance. When provided, chunk fitting is dispatched through this executor instead of the internal ProcessPoolExecutor, enabling callers to supply e.g. a submitit SLURM executor without drevalpy itself depending on submitit.
Replace `total_curves // cores` chunk sizing with an explicit `max_chunk_size` cap so that large dose-range groups are always split into manageable pieces and small groups are never inflated into oversized chunks. Rename `cores` to `max_workers` in `_run_work_items` and `fit_groups` to reflect that it controls local process pool width, not chunk granularity.
Repowise put hotspot health at 5.86 with 11 files in the alert band, seven of them literature predictors whose _fit methods were mostly duplicated torch scaffolding. Extracting that scaffolding is what the score was pointing at: _lightning_training.py and _early_stopping.py now hold the trainer assembly and the hand-rolled early-stopping loop that MOLIR, SuperFELTR, PharmaFormer and DIPK each reimplemented, and _omics_loaders.py, _pair_predict.py and _single_drug_omics.py hold the loader, predict and per-drug paths. DIPK's _fit goes 132 lines to 28. DRPModel and Featurizer each carried four unrelated concerns, so both split along their cohesion seams into mixins, and TunableComponentMixin in contracts/hyperparameter_space.py closes the last duplication that spanned featurizers and predictors. Three latent bugs fell out of the deduplication. The sklearn set_state assigned the estimator before validating the mode, so a malformed state left a half-updated predictor. SRMF built its response matrix with a running (existing + new) / 2 average, wrong for three or more duplicates, then overwrote every cell with a correct grouped mean. And two DRPModel methods had no callers and no tests at all. tools/size_gate.py is the ratchet, and it deliberately does not gate on repowise health: that score reads 10.0 on the fetch-depth 1 checkout actions/checkout does by default, because every commit-history marker vanishes with the history. A statement ceiling per module is computable from the working tree alone, so it means the same thing locally and in CI, and it is what keeps the mixin splits from growing back. Two coverage exemptions retired on measured values; no floor was raised. Also folds in the pyproject comment condensing that was already in the tree.
Rename the source from orakl_v2 to orakl and update the URL to s3://orakl-open-source-data/drevalpy_h5mu/gen3. Update tests that asserted on the old source name.
The refactor in c001f58 replaced the `cores` parameter with `max_chunk_size` and `max_workers` and renamed `chunk_size` to `max_chunk_size`, but the tests were not updated accordingly.
Picks up the n_cores<=1 pool short-circuit and df.copy() defragmentation that eliminate SLURM zombie processes and O(n^2) wide-DataFrame slowness.
|
|
||
| ## Contact | ||
|
|
||
| Main developers: |
Collaborator
There was a problem hiding this comment.
can you add yourself here pls? :D
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
2 participants
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.
This PR contains the current draft of v2.0.0 of drevalpy. It comes with the following changes:
What is currently missing: