Repository navigation
Orf benchmark - #25
ShinThomas wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Documented installations omit the new runtime dependency, and benchmark validation and report generation contain correctness issues.
5 open findings
What changed in this PR
Adds the V2 ORFfinder adapter and reproducible benchmark addressing issue #18. The benchmark finds all 1,784 NCBI candidates in adapter output, plus 129 adapter-only candidates.
Changes:
- Implements and tests corrected ORF translation.
- Adds benchmark tooling, fixtures, SCC execution, and provenance checks.
- Commits generated comparisons and documentation.
| File | Description |
|---|---|
src/lverage/orf_searcher.py |
Implements the ORFfinder adapter. |
tests/v2/test_orf_searcher.py |
Tests adapter and pipeline integration. |
pyproject.toml |
Pins orffinder==1.8. |
docs/api/orf_searcher.rst |
Documents adapter behavior. |
docs/singularity_setup.md |
Documents benchmark environment. |
benchmarks/orf_finder/.gitignore |
Ignores Python cache files. |
benchmarks/orf_finder/README.md |
Documents benchmark methodology and results. |
benchmarks/orf_finder/benchmark.py |
Implements comparison and reporting. |
benchmarks/orf_finder/run.qsub |
Runs the benchmark on SCC. |
benchmarks/orf_finder/tests/test_benchmark.py |
Tests parsing, coordinates, and reports. |
benchmarks/orf_finder/data/assigned_input_manifest.tsv |
Records input provenance and checksums. |
benchmarks/orf_finder/data/controls.fasta |
Provides synthetic controls. |
benchmarks/orf_finder/data/test_inputs/HoxA13.fa |
Supplies the HoxA13 input. |
benchmarks/orf_finder/data/test_inputs/Jun.fa |
Supplies the Jun input. |
benchmarks/orf_finder/data/test_inputs/MITF.fa |
Supplies the MITF input. |
benchmarks/orf_finder/results/candidates.tsv |
Records candidate-level results. |
benchmarks/orf_finder/results/comparisons.tsv |
Summarizes per-input comparisons. |
benchmarks/orf_finder/results/control_candidates.tsv |
Records control candidates. |
benchmarks/orf_finder/results/control_comparisons.tsv |
Summarizes control comparisons. |
benchmarks/orf_finder/results/matches.tsv |
Records exact and IoU matches. |
benchmarks/orf_finder/results/metadata.json |
Captures versions, commands, and provenance. |
benchmarks/orf_finder/results/overlaps.tsv |
Records candidate overlaps. |
benchmarks/orf_finder/results/report.md |
Presents generated findings. |
benchmarks/orf_finder/results/translation_differences.tsv |
Compares raw and corrected translations. |
benchmarks/orf_finder/results/unmatched.tsv |
Lists unmatched candidates. |
benchmarks/orf_finder/results/raw/assigned_inputs.fasta |
Stores combined benchmark input. |
benchmarks/orf_finder/results/raw/controls_ml30_nfalse_outfmt0.fasta |
Stores control proteins. |
benchmarks/orf_finder/results/raw/controls_ml30_nfalse_outfmt1.fasta |
Stores control CDS output. |
benchmarks/orf_finder/results/raw/controls_ml30_ntrue_outfmt1.fasta |
Stores nested-filter output. |
benchmarks/orf_finder/results/raw/controls_ml75_nfalse_outfmt0.fasta |
Stores thresholded proteins. |
benchmarks/orf_finder/results/raw/controls_ml75_nfalse_outfmt1.fasta |
Stores thresholded CDS output. |
benchmarks/orf_finder/results/raw/controls_python_package_loci.json |
Stores raw control loci. |
benchmarks/orf_finder/results/raw/ncbi_outfmt0_proteins.fasta |
Stores NCBI proteins. |
benchmarks/orf_finder/results/raw/ncbi_outfmt1_cds.fasta |
Stores NCBI CDS calls. |
benchmarks/orf_finder/results/raw/python_package_loci.json |
Stores Python package loci. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| license = {file = "LICENSE"} | ||
| dependencies = [ | ||
| "biopython>=1.85", | ||
| "orffinder==1.8", |
| # NCBI protein headers encode zero-based inclusive endpoints in either | ||
| # orientation; the CDS FASTA's 1-based inclusive range is authoritative. | ||
| if {encoded_coords[0], encoded_coords[1]} != {start0, end0 - 1}: | ||
| raise ValueError(f"NCBI protein/CDS coordinate encodings disagree for {tag}") |
| f"- Every NCBI candidate had an exact adapter match. The adapter returned {adapter_total - ncbi_total:,} additional candidates.", | ||
| "- All three inputs share at least one exact longest-ORF interval and strand between the adapter and NCBI.", |
| "biological_scope": "ORF scanning does not perform splicing and does not establish the protein annotated for the named gene", | ||
| "coordinate_system": "zero-based half-open on original input; strand separate", | ||
| "ncbi_terminal_stop_normalization": "CDS FASTA sequence/location includes stop; protein excludes translated terminal stop; normalized interval retains the terminal stop nucleotide", | ||
| "overlap_rule": "All same-strand positive non-exact interval intersections are emitted with IoU. Exact interval-and-strand matches are separate.", |
| - The runner, parser/coordinate tests, SCC job script, input provenance/checksums, | ||
| and generated outputs are under `benchmarks/orf_finder/`. New assigned-input | ||
| results remain pending until the compute-node benchmark runs. |
|
Addressed the five review findings:
|
|
@ShinThomas Run this command on your side: git checkout orf-benchmark This will remove the test inputs and results from the branch. It'll still be in the history but I will squash and merge it so that isn't retained. Data should not typically go into a repo unless said repo is specifically for that (or for test examples for end-use case but we aren't there yet). |
Remove every user- and machine-specific path from the benchmark so it runs from any checkout and any environment: - benchmark.py: drop the hard-coded ORFfinder path. The binary is resolved from --executable, ORFFINDER_EXECUTABLE, or an ORFfinder command on PATH, and a missing binary fails before any work starts. Add --input-dir (LVERAGE_ORF_BENCHMARK_INPUTS) and --results-dir so the unversioned FASTA inputs and generated outputs can live outside the repository. The provenance manifest is now checked for any input directory, not only the default one. - run.qsub: no default conda env, binary, or clone. Run from the submission directory, require ORFFINDER_EXECUTABLE (or PATH), activate CONDA_ENV only if given, and prepend the checkout's src/ to PYTHONPATH so the adapter under test is the one imported rather than another editable install. - README / singularity_setup.md: document the environment variables and the editable-install trap; remove personal paths. - tests: skip the real-input test when the FASTA files are absent; add tests for executable resolution and manifest enforcement.
8dc6d42 to
81e7241
Compare
- unmatched.tsv now records which pairwise comparison each unmatched candidate came from (comparison, unmatched_against), so a candidate appearing twice is explainable. - Replace the module-level raw-loci dictionary, which run_control_probes saved, cleared and restored by hand, with a dict passed explicitly to python_raw_candidates. - Surface NCBI ORFfinder stderr when a command fails instead of only the exit status. - README: note that the runner is deliberately pinned to the three assigned files and describe the new unmatched.tsv columns.
The benchmark should read the same on any machine and for any user: - No references to BU or the SCC. run.qsub is now a generic Grid Engine script: scheduler project flags are left to the qsub command line, and the conda module is loaded only if CONDA_MODULE is set. The README describes submission on any SGE cluster, or a plain python -m run elsewhere. - The input FASTAs are described as test inputs, not "professor-provided" or "assigned" files. Identifiers, the manifest filename, the generated report text, metadata keys and test names follow the same wording.
|
Hey Thomas, I pushed a few commits on top of yours. Quick rundown: Made the ORF benchmark self-contained. Everything that pointed at your account is gone: the ORFfinder path, your conda env, and your clone. The script now takes --executable, --input-dir, and --results-dir (or the ORFFINDER_EXECUTABLE / LVERAGE_ORF_BENCHMARK_INPUTS env vars). One thing to watch: your conda env has Lverage installed in editable mode from your clone, so running the tests from any other checkout was silently importing your adapter. The README now says to pip install -e . from the checkout you're testing or use PYTHONPATH=src. The real-input test also skips when the FASTA files aren't there, so the suite stays green once you remove the data. Tidied the ORF benchmark runner. Smaller stuff. unmatched.tsv now says which comparison each row came from, the module-level raw-loci global is gone (it's just passed in now), and a failing ORFfinder call shows its stderr instead of just an exit code. Removed site-specific and provenance wording. Nothing in the repo should assume BU or the SCC, and nothing should say where the test files came from. run.qsub is now a generic Grid Engine script (project flags go on the qsub command line, conda module only loads if you set CONDA_MODULE). "Assigned" and "professor-provided" are gone everywhere, including variable names, the report text, and the manifest file, which is now test_input_manifest.tsv. They're just test inputs. Going forward, let's keep that kind of context out of code and docs. What I need from you:
Comment here when that's done and I'll do a final pass. |
Summary
Addresses #18 by comparing
orffinder==1.8, Lverage’s corrected adapter from PR #24, and NCBI ORFfinder 0.4.3 on the suppliedHoxA13.fa,Jun.fa, andMITF.fagenomic sequences.Findings
Changes
Testing