diff --git a/DevDoc/architecture/numa_partition_runner/index.html b/DevDoc/architecture/numa_partition_runner/index.html index c25fea11..c79dca08 100644 --- a/DevDoc/architecture/numa_partition_runner/index.html +++ b/DevDoc/architecture/numa_partition_runner/index.html @@ -1367,6 +1367,17 @@ + + +
  • + + + + Panic propagation (fixed, 2026-08-28) + + + +
  • @@ -1553,6 +1564,17 @@ +
  • + +
  • + + + + Panic propagation (fixed, 2026-08-28) + + + +
  • @@ -1836,11 +1858,27 @@ dormant workers have been woken and grows every node by the same amount per step, capped by that node's remaining dormant workers and by the run's total budget (n_total) — balance across nodes is now guaranteed by construction, not incidental to channel implementation details.

    +

    Panic propagation (fixed, 2026-08-28)

    +

    A worker whose f(i) panicked used to never send its WorkerEvent::Completed +— the controller's while completed < n_total loop then waited forever for +an event that partition could no longer produce, since other live +workers/timer threads kept event_rx open. Discovered via obikselect +panicking on an unimplemented Sparse matrix case (see implementation/select.md): +the process hung indefinitely instead of erroring out.

    +

    Fixed: each worker wraps f(i) in std::panic::catch_unwind and sends a new +WorkerEvent::Panicked(i, payload) instead of silently dropping the +partition. The controller counts it toward completed (unblocking the loop) +and keeps the first payload seen; once run returns, panic::resume_unwind +re-raises it on the caller's thread — the original message/backtrace still +surfaces, from the right place, instead of a silent deadlock. Takes priority +over a plain Err (a panic means a bug, not a normal typed failure).

    Open questions

    @@ -1598,6 +1609,17 @@ +
  • + +
  • + + + + Known issue (not yet fixed, 2026-08-28) + + + +
  • @@ -1795,6 +1817,14 @@ spectrums/ <label>.json ← one file per genome, rebuilt from all sources index.meta ← complete genome list + evidence kind written at bootstrap +

    mphf.bin/unitigs.bin/evidence.bin/unitigs.bin.idx/fingerprint.bin/layer_meta.json marked "unchanged" above are hard-linked from the base source's own files during the bootstrap copy (2026-08-28), not copied — merge_partition never rewrites them for pre-existing layers, only the presence/counts subdirectory gets widened in place, so only that subdirectory is a real, independent copy. Falls back to a real copy per file if linking itself fails (different filesystems). Verified: source files are byte-identical (checksummed) before/after a normal merge.

    +

    Known issue (not yet fixed, 2026-08-28)

    +

    Merging an index against itself — literally the same directory passed twice as separate source arguments (e.g. obikmer merge -o out IDX IDX --rename-duplicates) — panics deep in the MPHF's rank-select structure (common_traits::select_in_word, assertion failed: rank < self.count_ones()), inside MphfLayer::find called from merge_partition's "is this source kmer already in dst" check against the bootstrap-copied dst_layers. Root cause not identified; ruled out so far:

    + +

    Only reproduces when sources[0] and sources[1] are the exact same on-disk path opened as two independent KmerIndex handles — an artificial scenario (nobody merges an index with itself intentionally), deprioritized rather than investigated further for now. Does terminate cleanly rather than hang, since PartitionRunner's panic propagation fix (see architecture/numa_partition_runner.md) now surfaces it as a normal process panic/exit 101 instead of a silent deadlock.

    diff --git a/DevDoc/implementation/select/index.html b/DevDoc/implementation/select/index.html index 97f4f7c1..c98f84e1 100644 --- a/DevDoc/implementation/select/index.html +++ b/DevDoc/implementation/select/index.html @@ -1119,17 +1119,6 @@ - - -
  • - - - - In-place: keep only group A - - - -
  • @@ -1157,6 +1146,23 @@ + +
  • @@ -1638,17 +1644,6 @@ - - -
  • - - - - In-place: keep only group A - - - -
  • @@ -1676,6 +1671,23 @@ + +
  • @@ -1728,23 +1740,32 @@ are preserved unchanged; only the data matrices are rewritten.


    Synopsis

    obikmer select <input-index>
    -        { --output <dir> | --in-place }
    +        --output <dir>
             [--group    <name>:<pred>  ...]
             [--group-op <name>:<op>    ...]
             [--aggregate-by <key>          ]
             [--aggregate-op <op>           ]
             [--select   <col1,col2,...>    ]
             [--presence-threshold <N>      ]
    +        [--force-copy                  ]
     

    Output destination

    -

    Exactly one of --output or --in-place must be specified.

    -

    --output <dir> — writes a new index to <dir>. The source index is -unchanged. The MPHF and unitig files are copied; only the data matrices are -rewritten with the new column layout.

    -

    --in-place — rewrites the data matrices of the source index directly. -Removed or replaced columns are lost. The operation writes to temporary files -first, then renames atomically, so an interrupted run leaves the index intact.

    +

    --output <dir> is required — select always writes a new index; there is no +--in-place mode (2026-08-28: never implemented, removed from the design). The +source index is unchanged.

    +

    Each layer's kmer-identity files (mphf.bin/unitigs.bin/evidence.bin/ +unitigs.bin.idx/fingerprint.bin/layer_meta.json) are never rewritten by a +column projection/aggregation, so they are hard-linked into the output rather +than copied — no extra disk for them even on a large index. Falls back to a +real copy automatically if linking fails (different filesystems); --force-copy +forces a real copy always, for an output that must survive independently of the +source on disk (a hard link shares the same inode — rewriting one path outside +select itself would affect the other). Only the presence/counts +subdirectory is ever a genuinely new, independent file.

    +

    To replace an index with a selected version of itself, select to a temporary +directory and swap it in (rm -rf INDEX && mv INDEX.tmp INDEX) — the case +--in-place used to cover.


    Defining output columns

    Named groups — --group

    @@ -1908,9 +1929,6 @@ is > N.

    obikmer select myindex --output out \
       --select "Betula_nana--TROM-V-149986,Betula_nana--AG-P04-25-01"
     
    -

    In-place: keep only group A

    -
    obikmer select myindex --in-place --group "A:group=A" --select "A"
    -

    Compose with filter

    # Step 1: keep only B. nana-specific k-mers
     obikmer filter myindex --output filtered \
    @@ -1921,32 +1939,46 @@ obikmer select 

    Implementation notes

    -

    select does not rebuild the MPHF. The 256 partitions are processed in parallel -(rayon), each writing its output independently; results require no synchronisation -because every partition owns a distinct set of files.

    -

    For each layer in each partition:

    +

    select does not rebuild the MPHF. Every partition is processed independently +(PartitionRunner), each writing its own output layers; no cross-partition +synchronisation is needed.

    +

    For each layer in each partition (obikselect::select_layer::select_partition):

      -
    1. The slot count n is read by opening the source data matrix.
    2. -
    3. A new data matrix is built with M columns (M = number of output columns).
    4. -
    5. For each slot s in 0..n:
    6. -
    7. old_row = matrix.fill_row(s) — reads the original N-column row without allocating.
    8. -
    9. For each output column j:
        -
      • new_row[j] = aggregate(op, old_row[group_indices]).
      • -
      • Pass-through columns are represented as single-element groups with the - default operator (any for presence, sum for count) — same code path.
      • -
      -
    10. -
    11. The new row is written slot by slot into each column builder.
    12. -
    13. All plain files in the source layer directory (mphf.bin, unitigs.bin, - evidence files, layer_meta.json) are copied verbatim; only the presence/ - or counts/ subdirectory is rewritten.
    14. +
    15. copy_layer_files hard-links the source layer's kmer-identity files + (mphf.bin/unitigs.bin/evidence.bin/unitigs.bin.idx/ + fingerprint.bin/layer_meta.json) into the destination — never a real + copy unless linking fails or --force-copy is given.
    16. +
    17. A new data matrix is built with M columns (M = number of output columns), + under a fresh presence//counts/ subdirectory (never touching the + source's own).
    18. +
    19. Presence source (2026-08-28: batch_presence_counts): one shared pass + over the source bit matrix computes every output group's presence count at + once — row-major native for a Sparse source (for_each_genome_in_row, + which has no column representation to read a col_view from at all — the + reason this replaced the old per-group loop, not just an optimisation of + it), deduplicated column-major (one col_view per distinct referenced + column, not per group) for Columnar/Packed. Every AggOp for a bit + matrix is then a cheap derivation of that one count vector (sum = the + count itself, any/max = count ≥ 1, all/min = count == group + size, none = count == 0) — see + obikselect::select_layer::agg_result_from_count.
    20. +
    21. Count source: unchanged, one col_view-driven pass per output column + via MatrixGroupOpssum/min/max are genuine per-value reductions + for a count matrix, not derivable from a single presence count the way + they are for a bit matrix.
    22. index.meta is rewritten with the new genome list and updated with_counts.
    -

    --in-place write strategy: new data is written to a temporary sibling -directory (presence_new/ or counts_new/); on success the old directory is -removed and the temporary one is renamed into place. An interrupted run leaves -at most one stale *_new/ directory; the original data is intact until the -rename step.

    +

    Known gap (not yet fixed, 2026-08-28)

    +

    Step 4 above still panics (col_view() not available on Sparse +PersistentCompactIntMatrix) if the source is a count index packed +sparse — batch_presence_counts' row-major treatment was only ported to the +bit-matrix (Presence) case, since that was the one actually blocking a real +benchmark run. select/filter on a sparse-packed count index still hits +this; the fix would follow the same shape (a PersistentSparseCompactIntMatrix +row-major decode, analogous to for_each_genome_in_row), just not done. Since +obisys::numa::runner::PartitionRunner's panic-propagation fix (see +architecture/numa_partition_runner.md), this at least fails fast (process +panic, exit 101) instead of hanging.

    diff --git a/DevDocMD/architecture/numa_partition_runner.md b/DevDocMD/architecture/numa_partition_runner.md index 06b7765c..994e8c76 100644 --- a/DevDocMD/architecture/numa_partition_runner.md +++ b/DevDocMD/architecture/numa_partition_runner.md @@ -304,10 +304,29 @@ step, capped by that node's remaining dormant workers and by the run's total budget (`n_total`) — balance across nodes is now guaranteed by construction, not incidental to channel implementation details. +## Panic propagation (fixed, 2026-08-28) + +A worker whose `f(i)` panicked used to never send its `WorkerEvent::Completed` +— the controller's `while completed < n_total` loop then waited forever for +an event that partition could no longer produce, since other live +workers/timer threads kept `event_rx` open. Discovered via `obikselect` +panicking on an unimplemented `Sparse` matrix case (see `implementation/select.md`): +the process hung indefinitely instead of erroring out. + +Fixed: each worker wraps `f(i)` in `std::panic::catch_unwind` and sends a new +`WorkerEvent::Panicked(i, payload)` instead of silently dropping the +partition. The controller counts it toward `completed` (unblocking the loop) +and keeps the first payload seen; once `run` returns, `panic::resume_unwind` +re-raises it on the caller's thread — the original message/backtrace still +surfaces, from the right place, instead of a silent deadlock. Takes priority +over a plain `Err` (a panic means a bug, not a normal typed failure). + ## Open questions - **Error handling**: `run` currently returns the first error; remaining errors - are dropped. A `Vec` return would give complete diagnostics. + are dropped. A `Vec` return would give complete diagnostics. (Panics are + now caught and re-raised individually — see above — this only concerns + plain `Err` results.) - **`INITIAL_DIVISOR` / `GROWTH_DIVISOR` tuning**: currently `4` and `8` (start at 1/4 of a node's cores, grow by 1/8 per step), chosen to fix an diff --git a/DevDocMD/implementation/merge.md b/DevDocMD/implementation/merge.md index cd273495..b93332d6 100644 --- a/DevDocMD/implementation/merge.md +++ b/DevDocMD/implementation/merge.md @@ -194,3 +194,14 @@ spectrums/