Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the chunked array downsampling logic that caused broadcast errors when dealing with half-integer scaled dimensions. The fix aligns the output shape calculation with scikit-image's rescale function by using np.round instead of np.ceil, and adapts the chunk placement logic to handle the actual downsampled chunk shapes.
Changes:
- Fixed output shape calculation in
_load_array_chunkedto usenp.roundinstead ofnp.ceil, matching skimage's internal behavior - Updated chunk placement logic to derive output slice bounds from actual downsampled chunk shapes rather than predicted sizes
- Added comprehensive regression tests covering single-chunk and multi-chunk scenarios for both instance and semantic data types
- Added
writer.flush()calls after figure logging in train.py for better data persistence - Code formatting improvements in train.py for readability
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/cellmap_segmentation_challenge/utils/matched_crop.py | Fixed rounding bug in output shape calculation (line 191) and chunk placement logic (lines 244-256) to prevent broadcast errors |
| tests/test_matched_crop.py | Added three comprehensive regression tests covering half-integer rounding scenarios for instance/semantic classes and multi-chunk processing |
| src/cellmap_segmentation_challenge/train.py | Added writer.flush() calls after add_figure for better reliability and improved code formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| out_z_end = min(int(np.ceil(z_end * scale_factors[0])), output_shape[0]) | ||
| out_y_start = int(y_start * scale_factors[1]) | ||
| out_y_end = min(int(np.ceil(y_end * scale_factors[1])), output_shape[1]) | ||
| out_x_start = int(x_start * scale_factors[2]) | ||
| out_x_end = min(int(np.ceil(x_end * scale_factors[2])), output_shape[2]) |
There was a problem hiding this comment.
The variables out_z_end, out_y_end, and out_x_end computed here are never used. The actual end positions are calculated later (lines 248-250) based on the downsampled chunk shape. These lines can be removed to avoid confusion and reduce unnecessary computation.
No description provided.