Skip to content

Add --progress[=INT] to view, norm and mpileup (periodic CHR:POS on stderr) - #2589

Merged
pd3 merged 4 commits into
samtools:developfrom
carstenerickson:progress-meter
Sep 10, 2026
Merged

Add --progress[=INT] to view, norm and mpileup (periodic CHR:POS on stderr)#2589
pd3 merged 4 commits into
samtools:developfrom
carstenerickson:progress-meter

Conversation

@carstenerickson

Copy link
Copy Markdown
Contributor

Resolves the simple version of #2559, as suggested by @pd3 there: a periodic
CHR:POS line on stderr so that long-running commands can be told apart from
hung ones. Nothing more (no percentages, no ETA, no byte counts), so region lists
and streamed input need no special handling and htslib is not touched. index
is intentionally out of scope (it would need an htslib hook).

Behaviour

  • --progress[=INT] on view, norm and mpileup. Optional argument, attached
    with = like -W, --write-index[=FMT]. Default 60 seconds.
  • One line per update, e.g. [progress] mpileup: 17:1234567. The command name is
    included so that view --progress | norm --progress remain distinguishable on a
    shared stderr; POS is 1-based.
  • A line is printed for the first record (so the user can see the option took
    effect and where the run started), then about once every INT seconds (the
    throttle uses time(), so the spacing is INT seconds to within a second).
    --progress=0 prints at every record; that is what the tests use.
  • Throttling is time based (one time() call per record when enabled, a single
    NULL test when disabled). A record-count throttle would give no heartbeat in the
    slow-per-record cases (deep pileup, realignment) that the option exists for.
  • view/norm report the input record's position (norm: before splitting or
    realignment); mpileup reports the pileup column at the top of the mpileup_reg()
    loop, before the region/BED filtering, so the meter keeps ticking while
    off-target columns are skipped (the man page says positions outside the
    requested regions or targets may be reported). Input decoding and pileup run
    on the main thread in mpileup, so no locking is needed.

Implementation

  • progress_init() / progress_update() in version.c, declared in bcftools.h next
    to the other shared command-line helpers; opaque struct, freed with free().
  • Six lines per command: struct field, usage line, option-table entry, case,
    the hook in the driver loop, and the free.
  • No API/ABI change, no new dependency.

Overhead (macOS, Apple Silicon, htslib develop): with the option enabled but never
firing, uncompressed VCF -> VCF passthrough of 4M records went from 1.39 s to 1.48 s
(the time() call per record; this is the worst case at ~2.8M records/s). With
bgzipped input the difference was within run-to-run noise. Disabled, there is no
measurable difference.

Docs and tests

  • doc/bcftools.txt entries for the three commands. doc/bcftools.1 and .html are
    not regenerated (they are normally refreshed by the maintainers).
  • test/test.pl: --progress=0 with stderr captured and filtered to [progress]
    lines for each command (expected files test/view.progress.1.out,
    test/norm.progress.1.out, test/mpileup/mpileup.progress.1.out), a bare
    --progress run covering the default path (test/view.progress.2.out, one
    line for the first record), and an expected-failure test for an invalid value.
    Full test suite passes on macOS (clang) and Ubuntu 22.04 (gcc 11), both
    against htslib develop.
  • NEWS entry in a separate last commit so it can be dropped or reworded easily.

Assisted-by: Claude:claude-fable-5-1

Long-running commands give no sign of life on stderr, so "still working"
cannot be told apart from "hung" without external tooling (samtools#2559). The new
optional argument prints the current CHR:POS for the first record and then
at most once every INT seconds (default 60; 0 prints at every record, which
the tests rely on). Nothing more elaborate is attempted, no percentages or
ETA, so that region lists and streamed input need no special handling and
htslib is not touched.

The small helper (progress_init/progress_update) lives next to the other
shared command-line helpers in version.c so that norm and mpileup can use it.

Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Carsten Erickson <carstene@gmail.com>
Same option and helper as in view. The position printed is that of the
input record, before splitting or realignment.

Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Carsten Erickson <carstene@gmail.com>
Same option and helper as in view and norm. The hook sits at the top of the
mpileup_reg() loop, before the region and BED filtering, so that the meter
keeps reporting while off-target columns are being skipped. Input decoding
and pileup run on the main thread (the thread pool only serves the output),
so no locking is needed.

Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Carsten Erickson <carstene@gmail.com>
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Carsten Erickson <carstene@gmail.com>
@pd3
pd3 merged commit 9c792a5 into samtools:develop Sep 10, 2026
16 checks passed
@pd3

pd3 commented Sep 10, 2026

Copy link
Copy Markdown
Member

This looks good. Thank you

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