fix(plotting): phase-lock add_volume's in-plane display grid across volumes - #393
Open
sdiebolt wants to merge 9 commits into
Open
fix(plotting): phase-lock add_volume's in-plane display grid across volumes#393sdiebolt wants to merge 9 commits into
sdiebolt wants to merge 9 commits into
Conversation
…olumes Each volume overlaid on a VolumePlotter previously resampled its two in-plane axes onto an independently-computed axis-aligned grid, with the origin taken from that volume's own world-space bounding box. Two volumes at the exact same resolution (e.g. a fixed/moving pair after rigid registration) could therefore land on grids offset from each other by a sub-voxel phase, even though the underlying registration was correct. Add snap_origin_to_phase, which shifts a later volume's own-resolution grid origin down (by less than one of its own voxels) so it's congruent with the first volume plotted, without moving it toward that volume's origin. Thread it through VolumePlotter's spatial-slice_mode, pose-faceted, and non-pose extra-dim resample paths via a new per-axis self._axis_origin_phase registry. Fixes #391.
This reverts commit e10f11d.
Renders a fixed volume with a rotated + non-integer-voxel-translated moving overlay (mirroring a rigid-registration fixed/moving pair), so the two volumes' cell boundaries in the overlap region are visible in the baseline image. Confirmed against the pre-fix commit that a phase mismatch shows as a jagged, offset seam between the two grids, versus a clean aligned seam here.
Contributor
|
📖 Doc preview: https://confusius.tools/pr-preview/pr-393/ |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
snap_origin_to_phase previously made a later volume's grid origin (a voxel center) congruent to the reference volume's own origin, modulo the later volume's spacing. That only guarantees coinciding cells when both volumes share the same spacing -- at any other resolution ratio (e.g. a higher-resolution moving overlay), every other fine-grid voxel center lands exactly on a coarse-grid edge instead of nesting inside a coarse cell, so the two grids' cell boundaries still don't align. Add AxisPhase (origin, spacing) and rework snap_origin_to_phase to align cell edges (origin - spacing/2) instead: the shared reference point is now derived from the reference volume's own edge, which requires knowing its spacing too. For matching resolutions this reduces to the previous math exactly (confirmed by the unchanged same-spacing baseline); for differing resolutions, this now nests the finer grid's cells inside the coarser one instead of merely locking their centers. Threaded AxisPhase through compute_oblique_axis_aligned_grid_geometry, resample_to_axis_aligned_world_grid, compute_shared_slice_axis_grid_geometry, _resample_to_shared_slice_axis_grid, _resample_to_planar_world_grid, and VolumePlotter._axis_origin_phase. Adds a differing-spacing visual regression baseline (test_plot_volume_overlay_different_spacing_moving) and a unit test for the edge-alignment behavior specifically.
Two independently axis-aligned volumes sharing a world origin (e.g. both zeroed to the same anatomical landmark, as with two BrainGlobe/OfUSA-style templates) but at different resolutions have matching voxel *centers* but not matching cell *edges*. Axis-aligned data was always returned unchanged by _resample_to_shared_slice_axis_grid/_resample_to_planar_world_grid regardless of phase state, so this case was never corrected even after the center-vs-edge fix, since that fix only ever ran on the oblique-resample path. Add _matches_phase to check whether an axis-aligned volume's own native grid already satisfies the established phase reference, and _native_axis_phases to compute its native origin/spacing without requiring every voxel dim to have regular spacing (the slice axis, never phase-locked, may be a non-contiguous voxel selection even on otherwise axis-aligned data). When a volume doesn't already match, it's now resampled (identity transform) even though axis-aligned, shifting its in-plane cells onto the phase-locked grid; its own slice-axis discretization still always stays native. Confirmed against real allen_mouse_100um atlas + Huang 2025 fUSI template data (both axis-aligned, exact 2x spacing ratio, shared world origin), which previously showed the moving volume's cells centered inside the fixed volume's cells rather than nesting at shared edges.
_resample_to_planar_world_grid's axis-aligned-but-phase-mismatched branch (added for the atlas/template fix) was uncovered: going through the full add_volume/plot_volume pipeline with a pose-faceted panel takes the oblique branch instead, since .isel(pose=...) can leave enough floating-point noise for has_axis_aligned_voxel_to_world_index to read False even on genuinely axis-aligned data. Call _resample_to_planar_world_grid directly so the branch is exercised deterministically.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
add_volumeoverlays a later volume's in-plane display grid independently of the first volume plotted on the sameVolumePlotter, using that volume's own bounding-box origin. Two volumes at the same resolution (e.g. a fixed/moving pair after registration) could land on grids offset by a sub-voxel phase, purely as a display-resampling artifact — not a registration error.snap_origin_to_phase(src/confusius/_utils/plotting.py) and threads it throughVolumePlotter's spatial-slice_mode, pose-faceted, and non-pose extra-dim resample paths via a newself._axis_origin_phaseregistry, so a later volume's grid is phase-locked to the first volume's — without moving its own origin anywhere near the first volume's (which may sit far away in world space).(origin - phase_origin) % spacingcan land nearspacinginstead of near0whenoriginis already an exact multiple away (1.0 % 0.2in Python), which previously shifted a degenerate single-point axis's origin by a spurious extra step.Fixes #391.