Skip to content

fix: remove the padding again in predict_instance_mask(pad_size=n) - #78

Open
Hendrik-code wants to merge 3 commits into
mainfrom
fix/instance-pad-size
Open

Hendrik-code wants to merge 3 commits into
mainfrom
fix/instance-pad-size

Conversation

@Hendrik-code

Copy link
Copy Markdown
Owner

Problem

# Uncrop again
if pad_size > 0:
    whole_vert_nii_uncropped.apply_pad(-pad_size)     # <- result dropped

NII.apply_pad has inplace=False, so the padding added at the top of predict_instance_mask was never
removed. On the shared test fixture:

pad_size=0 -> (68, 52, 67)
pad_size=4 -> (76, 60, 75)      # 2 * 4 larger per axis, and a shifted field of view

The docstring says the padding is "removed afterwards". predict_instance_mask is exported as
spineps.predict_instance_mask, so a caller using the documented parameter gets a mask that does not
line up with their input. The pipeline itself always passes the default 0, which is why this was never
noticed.

A second defect sat behind it: the padding was applied before the rescale to the model resolution, so
pad_size input voxels become a different number of voxels in the grid the un-pad operates on. Even a
correctly rebound apply_pad(-pad_size) would have removed the wrong amount whenever the input zoom
differs from the model's.

Fix

  • Rebind the result of the un-pad.
  • Pad after the rescale instead of before, which makes the removal an exact inverse at any input
    resolution.

pad_size=0 -- every call inside the pipeline -- is bit-for-bit unchanged: the existing
test_regression_golden digests still match.

Tests

unit_tests/test_instance_pad_size.py: pad_size=4 returns the same shape, affine and voxel content as
pad_size=0, and the same holds for a model whose resolution forces a rescale. All 3 fail on main.

Full suite: 226 passed, ruff clean.

🤖 Generated with Claude Code

Hendrik-code and others added 3 commits October 5, 2026 08:31
`whole_vert_nii_uncropped.apply_pad(-pad_size)` is not an in-place call and its result was
dropped, so any `pad_size > 0` returned a mask `2 * pad_size` voxels larger per axis than
the one handed in -- while the docstring promised the padding was "removed afterwards".
`predict_instance_mask` is part of the public API (`spineps.predict_instance_mask`); the
pipeline itself always passes the default 0, which is why this never surfaced.

The padding was also applied before the rescale to the model resolution, so even a
rebound `apply_pad(-pad_size)` would have removed the wrong number of voxels whenever the
input zoom differs from the model's. Padding now happens after the rescale, which makes
the removal its exact inverse. `pad_size=0` -- every call in the pipeline -- is bit-for-bit
unchanged (the golden regression digests still match).

Co-Authored-By: Claude Opus 5 <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