Skip to content

hierarchy: dedup _get_ref_coords gather + vectorize voxel→node adjacency#243

Merged
aelefebv merged 1 commit into
dechaofrom
hierarchy-perf-c-cleanups
May 12, 2026
Merged

hierarchy: dedup _get_ref_coords gather + vectorize voxel→node adjacency#243
aelefebv merged 1 commit into
dechaofrom
hierarchy-perf-c-cleanups

Conversation

@aelefebv

Copy link
Copy Markdown
Owner

Summary

Two safe-and-free cleanups in `hierarchical.py`:

  • `_get_ref_coords` redundant gather — `vals_a` and `vals_b` were computed by the same `idxmin[branch_labels_clipped]` expression (same input array, same lookup). Compute once, share the NaN mask. ~6 lines deleted; fewer allocations on the hot motility path (called per-frame inside `_get_motility_stats`).

  • `_save_adjacency_maps` voxel→node Python loop — replaces the nested `for voxel_idx, nodes in enumerate(...): for n in nodes: edges_vn.append((voxel_idx, int(n)))` with `np.repeat` + `np.concatenate`. Only fires when `not skip_nodes` (production default in the napari pipeline). Removes per-edge Python attribute + tuple-allocation overhead — a 1MB-edge frame goes from millions of Python ops to two C-level numpy calls.

Equivalence bar

Bit-identical. Verified locally on yeast 2D + 3D fixtures via SHA over all 6 hierarchy outputs (`features_voxels` / `features_nodes` / `features_branches` / `features_organelles` / `features_image` / `adjacency_maps`) — all 12 hashes match the post-PR-#242 baseline exactly.

Why

Closing out the 3-PR hierarchical perf audit (PRs #241, #242, this one). cProfile flagged `_save_adjacency_maps` and `_get_ref_coords` as small but allocation-heavy hot paths; this PR captures the trivial wins identified during the audit. No new features, no new tests required (existing 57 tests in `test_hierarchical.py` cover both code paths and pass; SHA verification proves bit-identity).

Test plan

  • Full `tests/test_hierarchical.py` (57 tests) passes.
  • Bit-identical SHA on 2D + 3D fixtures (local pre/post comparison).

🤖 Generated with Claude Code

Two safe-and-free cleanups in `hierarchical.py`:

- **`_get_ref_coords` redundant gather** — `vals_a` and `vals_b` were
  computed by the *same* `idxmin[branch_labels_clipped]` expression
  (same input array, same lookup). Compute once, share the NaN mask.
  ~6 lines deleted; fewer allocations on the hot motility path
  (called per-frame inside `_get_motility_stats`).

- **`_save_adjacency_maps` voxel→node Python loop** — replaces the
  nested `for voxel_idx, nodes in enumerate(...): for n in nodes:
  edges_vn.append((voxel_idx, int(n)))` with `np.repeat` +
  `np.concatenate`. Only fires when `not skip_nodes` (production
  default in the napari pipeline). Removes per-edge Python attribute
  + tuple-allocation overhead — a 1MB-edge frame goes from millions of
  Python ops to two C-level numpy calls.

Bit-identical: verified locally on yeast 2D + 3D fixtures via SHA
over all 6 hierarchy outputs (`features_*` + `adjacency_maps`) — all
12 hashes match the post-PR-#242 baseline exactly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@aelefebv
aelefebv merged commit 91cda7e into dechao May 12, 2026
6 checks passed
@aelefebv
aelefebv deleted the hierarchy-perf-c-cleanups branch May 12, 2026 03:29
aelefebv added a commit that referenced this pull request May 12, 2026
…ation audit close-out (#244)

- now.md: new "Recently shipped" entry covering PRs #241/#242/#243
  (vectorized per-label group construction, vectorized
  _get_branch_stats loops + ADR 0013, _get_ref_coords + vox→node
  cleanups). Watch list updated to reflect hierarchical's pinning
  status and the deferred wins (regionprops solidity ~17%,
  flow_interpolation _get_vector_weights ~28%) that the next perf
  audit could target.
- feature-extraction.md: new Performance section documenting the 4
  shipped optimizations + the 3 deferred items, plus a Benchmarks
  subsection pointing at tests/test_hierarchical_perf.py.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant