perf(qc): compress chopper output with bgzip (8x faster QC stage) + wait on every pipeline stage

[sanjaynagi-eit]

Hey @gbouras13 ! thank you for plassembler! I've been using it recently in a pipeline which I'm trying to optimise. I noticed the below and wondered whether you'd like to incorporate it, seemed worthwhile to me :)

---

chopper() builds the chain gunzip -c reads.gz | chopper --threads N | gzip. Every stage is parallel-aware except the last one, and gzip dominated the stage completely.

Measured on a real 300 MB ONT fastq, 8 threads:

chain	time	output
gunzip | chopper (no compression)	5.4 s	-
... | gzip (current)	47.7 s	293.4 MiB
... | bgzip -@ 8	5.9 s	276.7 MiB

So ~42 of the 47.7 seconds were serial gzip. On the same isolate the whole plassembler long run is 499 s, making this ~8% of total wall clock for free.

Suggested change

Use bgzip -@ threads as the compressor.

• No new dependency: bgzip ships with htslib/samtools, which plassembler already requires and already checks for in check_dependencies.
• No format change: BGZF is a valid gzip stream. flye, minimap2, chopper, gunzip and python's gzip module all read it unchanged. chopper_long_reads.fastq.gz is in any case a purely internal intermediate that remove_intermediate_files deletes.
• Falls back to plain gzip if bgzip is somehow absent.
• Here it also happens to be smaller (277 vs 293 MiB).

Verified content-identical: with chopper --threads 1 the decompressed output of the old and new chains is byte-for-byte equal. (Above 1 thread chopper does not preserve read order, so only the single-threaded comparison is meaningful — that is pre-existing chopper behaviour, not something this PR changes.)

gzip_file(), used by --skip_qc, went through python's gzip module — slower still — and now uses the same compressor with a python fallback.

Bug fix in the same function

Only gzip_proc was ever waited on. source_proc and chopper_proc were never wait()ed and the parent never closed its copies of their pipe read ends, so:

• a failing gunzip or chopper was silently swallowed (the bare except: never saw it, because the exception never happened — the process just exited non-zero), leaving an empty or truncated read file that the rest of the run happily consumed;
• zombie processes accumulated.

Every stage is now waited on and a non-zero exit is reported with the stage name and the log path. test_chopper_reports_a_failing_stage covers this: feeding plain fastq with gzip_flag=True used to pass silently and now raises. The uncompressed-input branch also no longer spawns a cat process just to copy the file into a pipe — the file is handed to chopper's stdin directly.

[gbouras13]

Hi @sanjaynagi-eit

Thanks for this, looks like a big cleanup and runtime improvements. Looks like it will be used on Linux only given the dependency tree - I am reticent to add some dependency origami to make it pinned for macOS too, it will introduce fragility.

There's also a new chopper parameter warning that the review detected, but I will fix that on a separate PR

I've had LLM-augmented review of your PR - looks good to me. The main thing is one revert needed in the test data if
you don't mind doing that before I merge this PR in :)

Thanks again mate

George

Claude

What I verified

The speedup is real — I measured a bit more than you did. 304 MiB ONT fastq, 8 cores:

	time	output
gzip	36.8s	161 MiB
bgzip -@ 8	3.6s	149 MiB

~10x, and smaller output. I also confirmed the BGZF output passes gunzip -t and reads back cleanly through Python's gzip module, so the "valid gzip stream" claim checks out.

The process-handling fix is the more valuable half of this PR, and I don't think the title gives it enough credit. Running chopper directly on main's input_half.fastq.gz:

thread '<unnamed>' panicked at src/main.rs:443:45:
ERROR: problem parsing fastq record: IncompleteRecord
exit code: 101

Because only gzip_proc was waited on, that panic was swallowed and the pipeline carried on with a truncated read set. So this isn't just tidier process handling — plassembler could silently proceed with partially-filtered long reads whenever chopper failed. That's worth a HISTORY.md entry.

Your fixture repair was correct. I checked: main's copy was 778 lines — 194 complete records plus a 2-line orphan — and yours is byte-identical for the first 776 and stops on a record boundary. Good catch, and thanks for explaining it in the commit message rather than just fixing it.

One thing to revert

tests/test_data/validation/concat.fastq is truncated to 0 bytes in 53f72f0 (104,636 → 0). I think this was accidental — it's unrelated to the rest of the commit.

I initially assumed a test had clobbered it, since test_concat_single_fastq_bad names it as an output path, but that's not it: concatenate_single_fastq consumes both inputs via records.extend(SeqIO.parse(...)) before opening the output, so the ValueError fires first and the file is never opened for writing. I restored it and ran the full non-slow suite — it stays at 104,636 bytes. So it looks like a stray shell redirect or editor mishap rather than anything systematic.

Nothing reads it as an input so it broke nothing, but it drops 100 KB of real ONT reads. I have the revert plus one small follow-up on a branch — happy to push them to your branch, or you can just git checkout main -- tests/test_data/validation/concat.fastq.

Heads-up on the bgzip dependency

bgzip ships with htslib/samtools, already a hard plassembler dependency, so this adds nothing to the environment

Almost — but it's transitive, and it doesn't hold on Apple Silicon. bioconda's samtools package ships only the samtools binary; bgzip comes from htslib. Our pixi.lock resolves to:

platform	htslib	bgzip
linux-64	1.23.1	yes
osx-64	1.23.1	yes
osx-arm64	none	no

Two consequences worth knowing: your benchmark won't reproduce on an M-series Mac (you'll silently get the gzip fallback), and the two skipif(shutil.which("bgzip") is None) tests skip on our macos-latest runners, so half the CI matrix never exercises the new path.

I looked at just adding htslib to the pixi deps, but it's a dead end for now — no osx-arm64 htslib build is compatible with the libdeflate 1.25 that fastp >=1.3 needs, and the solver fails outright. Downgrading fastp to 1.1.0 to gain bgzip would change short-read trimming behaviour on one platform only, which I'm not willing to do. So we're leaving it transitive: full speed on Linux and Intel Mac, graceful fallback on Apple Silicon. No change needed from you — just don't be surprised when it's not faster on an M-series machine.

One nit (already applied on my branch)

In the except OSError handler, the stages are kill()ed but never wait()ed, which leaks exactly the zombies the rest of the change removes. One line. I kept the return after logger.error — it looks dead because the CLI's ERROR sink exits, but qc.py is importable without that sink, and falling through would re-wait on the just-killed processes and report them a second time.

Separately (not yours)

While testing I noticed chopper 0.13.0 warns --headcrop is set but is ignored because no --trim-approach was selected. We pass --headcrop 75 --tailcrop 75 without --trim-approach fixed-crop, so our head/tail cropping is currently a silent no-op. I'll deal with that on my own.