Skip to content

Correct localize coordinates, validation, charts, and regional errors - #700

Open
SuhasSrinivasan wants to merge 8 commits into
nanoporetech:masterfrom
SuhasSrinivasan:codex/fix-localize-reporting-and-regions
Open

Correct localize coordinates, validation, charts, and regional errors#700
SuhasSrinivasan wants to merge 8 commits into
nanoporetech:masterfrom
SuhasSrinivasan:codex/fix-localize-reporting-and-regions

Conversation

@SuhasSrinivasan

Copy link
Copy Markdown
Contributor

Fixes #674.
Fixes #684.
Fixes #685.

Summary

  • Correct localize feature-window geometry and report reference-axis offsets from the original feature anchor.
  • Validate complete region files before creating output and propagate regional query failures from localize and stats.
  • Render chart values in percent units, support sparse one-offset data, and avoid creating/truncating chart files for empty data.
  • Document that affected historical localize output must be regenerated.

Severity

Severity: High — scientific correctness, input integrity, and output reliability

Rationale: The coordinate defect changes which observations enter feature profiles and places retained observations in incorrect bins. Invalid region rows and query errors could silently produce incomplete successful output. Chart scaling is Medium reporting impact but is part of the same localize output contract.

Root cause

Window construction reconstructed anchors from clipped intervals and used an asymmetric half-open range; offset subtraction had the opposite reference-axis sign. The region parser accepted mixed-validity input incrementally, regional Rayon folds discarded errors, and chart code plotted fractional values on a percent axis while assuming multiple x positions.

Implementation

  • Preserve the original anchor, query the symmetric half-open window [anchor-w, anchor+w+1), and emit bedMethyl position minus original feature anchor for all BED strand forms.
  • Parse and validate the complete BED3+ region file, including line context, numeric/strand fields, bounds, comments, and opaque extra fields, before opening output.
  • Replace infallible regional folds with contextual try_fold/try_reduce propagation while retaining missing-contig skipping.
  • Plot numeric percent values, pad singleton x bounds, show one-datum markers, and validate empty data before file creation.

Preserved behavior

  • Valid normalized inputs, missing-contig skips, output schemas, and thread 1/4 bytes remain unchanged except for the corrected coordinate/chart values.
  • Columns 7+ in region files remain opaque.
  • Rayon cancellation remains cooperative.

Non-goals

  • No feature-oriented strand reversal; offsets use one reference-axis contract.
  • No localize batch-size policy change.
  • No shared writer-finalization or general pipeline-cancellation redesign.

Behavior before and after

Case Before After Oracle
Interior anchor, window w Shifted/asymmetric membership and reversed offset [anchor-w, anchor+w+1), offset position-anchor Exact symmetric reference-axis bins
Edge-clipped feature Reconstructed anchor changed Original anchor retained Correct clipped membership and offsets
Mixed valid/invalid BED Partial successful aggregate possible Contextual nonzero error before output creation No incomplete success
Corrupt regional query Error could be skipped Contextual command failure No silent observation loss
TSV value 50 / sparse chart Chart plotted 0.5 or failed/left empty file Chart plots 50 and handles singleton data TSV/chart unit identity

Testing

Environment: macOS 26.6 arm64; rustc/cargo 1.90.0; installed modkit 0.6.4 reference; ignored test-only Cargo.lock SHA-256 78876ab4a98da30caad167744d1c8a0875c27edad7b0b3ad5f8a1891f78604ea resolving hts-sys 2.2.0. Cargo.lock is not in the diff.

  • Revision/tree: 2d68c7e / b85355138d8775667a35371022130f5cfc80804c; clean tracked worktree.
  • Parent-red fixtures reproduced shifted offsets/membership, ignored invalid rows, swallowed indexed-query failures, fractional chart values, and sparse/empty chart failures.
  • cargo test --offline --locked -p mod_kit localise -- --test-threads=1: 23 passed, 0 failed.
  • cargo test --offline --locked -p modkit --test test_localize -- --test-threads=1: 8 passed, 0 failed.
  • cargo test --offline --locked -p modkit --test test_stats -- --test-threads=1: 2 passed, 0 failed.
  • cargo test --offline --locked --workspace --all-targets -- --test-threads=1: 211 active tests passed, 14 declared ignored, 0 failed.
  • Parser matrix covered BED3-6+, blank/whitespace/comment lines, spaces in names, opaque/empty extra columns, score/strand/bounds failures, line-numbered diagnostics, absent-output and sentinel-preservation behavior.
  • Coordinate matrix covered window 0/2, coordinate zero, both edges, plus/minus/unstranded features, and exact offset bins.
  • Valid threads 1/4 artifacts were byte-identical; malformed regional-query fixtures failed without creating absent output or overwriting sentinels.
  • Chart regressions covered percent-unit identity, one datum, multiple series sharing one offset, empty data, multi-offset controls, and unchanged TSV bytes.
  • git diff --check upstream/master...HEAD passed; the worktree remained clean.
  • Repository-wide stable cargo fmt --all -- --check reports unchanged upstream formatting plus nightly-only settings; no unrelated rewrite was made.

Tests not performed

  • cargo clippy was not run.
  • The 1-GB direct-RNA slice was not used because exact synthetic bedMethyl/BED fixtures provide coordinate and failure oracles.
  • Performance/RSS benchmarking was not run.

Scientific validation

  • Every retained observation is assigned relative to the original feature anchor with one documented sign convention.
  • Invalid features or unreadable regional data cannot silently disappear from a successful aggregate.
  • Valid counts and percentages are conserved across thread counts.
  • TSV and chart represent the same percent quantity.

Output and compatibility

Reviewer guide

  1. Review window/offset construction in localise/subcommand.rs and util.rs.
  2. Review parser-before-output ordering and regional try_fold/try_reduce paths.
  3. Review the focused integration fixtures, then rerun the three commands above.

Checklist

  • All three issues contain reproducible observed and expected behavior.
  • The commits remain separated by coordinate, validation/error, documentation, and chart concerns.
  • Parent-red and fix-green evidence is recorded.
  • All material tests and omissions are listed.
  • Coordinate/count/chart contracts and deterministic controls are checked.
  • Diff hygiene passed; unrelated format findings are disclosed.
  • No private data, generated lockfile, or unrelated change is included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant