Skip to content

Fix stale tests and empty significant-programs crash - #15

Open
engreitz wants to merge 13 commits into
mainfrom
fix-stale-tests
Open

engreitz wants to merge 13 commits into
mainfrom
fix-stale-tests

Conversation

@engreitz

Copy link
Copy Markdown
Contributor

Base: scrub-lab-specific-content (not merged yet; it rewrites paths in these files). Retarget to main once it lands.

Problem

A full test run on an HPC cluster (main @ d5289bf, 2026-09-27) failed in four suites. The tests had drifted from the code, and one real bug turned up.

Fix

Area Failure Change
Stage 1 sk/torch expected loading/cNMF_scores_5_2.0.txt expect ..._5_2_0.txt (what compile_results writes)
Stage 1 parallel sk/torch rename_all_NMF found no spectra per-K runs use the {run}_{K}/Inference/cnmf_tmp layout that the pipeline uses
Stage 2 U-test (15) missing out_dir, run_name, K, sel_threshs call U_test functions with explicit args (no module-global args)
Stage 3 KSelection (5) load_perturbation_data(samples=) conditions=, condition column, *_per_condition / *_all_conditions plot names
Stage 3 Gene (5) isinstance(DataFrame), DID NOT RAISE assert LazyGeneCorr / LazyPerturbCorr and check rows against the dense Pearson matrix; pass gene_name_key='symbol'
Bug: get_significant_programs_df KeyError: 'target_name' when nothing is significant build the DataFrame with explicit columns; new unit test

Result (cluster, SLURM)

Suite Result
torch batch / parallel 2 / 2 passed
sk-cNMF + parallel 11 passed
Stage 2 53 passed, 29 skipped*
Stage 3 103 passed, 5 skipped*
Excel summary CLI on the mini run now completes (crashed before)

* Skips are CRT tests (skipped on main as well) plus tests that need untracked resources (GWAS file, guide annotation). Rerun with those resources present: test_metrics + test_utest 61 passed, 0 skipped; KSelection 20 passed, 0 skipped. The only remaining skips are the CRT tests.

🤖 Generated with Claude Code

engreitz and others added 13 commits September 27, 2026 22:12
Replace hardcoded cluster paths, partitions, email and study-specific
inputs in the SLURM .sh runners with placeholders and a required
PIPELINE_ROOT env var. Move commands that preceded the #SBATCH block
below it so sbatch parses the directives.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
generate_slurm.py now derives the repo root from its own location (or
PIPELINE_ROOT), takes partition/email from args or env with no baked-in
default, and only writes GPU feature constraints when asked. Skill docs
ask the user for partition, email and reference paths instead of
assuming one cluster.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace sys.path entries pointing at one cluster checkout with paths
computed from each script's location, load evaluation resources from the
repo's Resources/ dir, make the mini-dataset builder take its input as a
CLI argument, and drop cluster paths from test docstrings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add a public-repository rule section to CLAUDE.md, replace the cluster
environment and resource-path blocks with generic PIPELINE_ROOT /
Resources wording, and make setup_resources.sh take its source dir from
RESOURCES_SRC.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Delete skill evaluation/workspace outputs that recorded study runs, and
gitignore tasks/, .baton/ and skill workspace/output dirs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace cell-type-specific defaults in the annotation code (prompt role
and context, PubMed keyword, evidence-scoring keywords) with generic
ones; the PubMed keyword is now optional. Vertex AI project, location
and bucket come from env vars or CLI with no built-in project.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tools/check_no_lab_specific_content.py scans tracked files (or staged /
ref-relative added lines) for forbidden patterns, with exceptions in
tools/lab_specific_allowlist.txt. Runs in CI on push/PR and as a local
pre-commit hook. CLAUDE.md points to it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
--Conditions (plotting) and --Sample (excel summary) no longer default
to one study's labels; when omitted, labels are the unique values of the
categorical key in the h5mu. Runners stop passing hardcoded labels and
docs use placeholder labels.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Excel summary's --Sample flag is now --Conditions (--Sample kept as
a deprecated alias with a notice). --categorical_key and --Conditions
help texts are unified across stages, docs and the runner skill use the
condition wording, and the drift check understands flag aliases.
CHANGELOG records the rename and the scrub changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Stage 1: expect loading/cNMF_{scores,loadings}_{K}_{thresh with _}.txt,
  matching compile_results; parallel tests now use the
  {run}_{K}/Inference/cnmf_tmp layout that rename_all_NMF reads.
- Stage 2 U-test: call U_test functions with explicit
  out_dir/run_name/K/sel_threshs instead of a module-global args.
- Stage 3 KSelection: load_perturbation_data(conditions=...), 'condition'
  column, *_per_condition / *_all_conditions plot names.
- Stage 3 Gene: pass gene_name_key='symbol'; assert LazyGeneCorr /
  LazyPerturbCorr and check rows against the dense Pearson matrix.
- get_significant_programs_df: keep columns when nothing is significant
  instead of raising KeyError on set_index; add unit test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Compile_excel_sheet.py: summary functions take conditions= (matching
  load_perturbation_data and --Conditions); callers, README, notebook and
  tests updated. simple_Summary_cols locals renamed so they no longer
  shadow the parameter.
- get_significant_programs_df: '# programs <condition>' is always int
  (was str counts mixed with int 0); test checks dtype and values.
- Gene corr test: compare only uniquely named genes.
- KSelection conftest: drop unused synthetic_test_stats_df fixture.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@ymo6
ymo6 changed the base branch from scrub-lab-specific-content to main September 30, 2026 01:18
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.

2 participants