Skip to content

Fix detach_sample dropping channels of multi-channel views#457

Merged
srivarra merged 2 commits into
mainfrom
fix/log-images-multichannel
Jun 5, 2026
Merged

Fix detach_sample dropping channels of multi-channel views#457
srivarra merged 2 commits into
mainfrom
fix/log-images-multichannel

Conversation

@edyoshikun

Copy link
Copy Markdown
Member

Summary

detach_sample (in viscy-utils) computed a single n_channels from the first view and applied it to all views. For virtual staining where source is 1 channel but target/pred are 2 channels (nucleus + membrane), the second channel was silently dropped — so TensorBoard train_samples / val_samples grids showed only one fluorescence channel instead of both.

This is the logging path used by cytoland.engine.VSUNet (and dynacell), which is what the image_translation course exercise exercises.

Root cause

n_channels = imgs[0].shape[1]      # source = 1 channel
for img in imgs:
    for c in range(n_channels):    # applied to target/pred too -> 2nd channel dropped

Introduced in the monorepo refactor (#373); the pre-split detach_sample kept each view's own channels.

Fix

Loop over each view's own channel count (range(patch.shape[0])). Views with differing channel counts are all logged in full.

Verification

  • New regression test log_images_test.py: phase2fluor shapes (source 1ch, target/pred 2ch) now produce 5 columns per row (1 + 2 + 2); rendered grid is (192, 320, 3).
  • uv run pytest packages/viscy-utils/src/viscy_utils/log_images_test.py passes.
  • Ruff clean.

🤖 Generated with Claude Code

detach_sample computed a single n_channels from the first view
(source) and applied it to all views. For virtual staining where
source is 1 channel but target/pred are 2 channels (nucleus +
membrane), the second channel was silently dropped, so TensorBoard
sample grids showed only one fluorescence channel.

Loop over each view's own channel count instead. Add a regression
test and update the docstring to cover views with differing channel
counts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes viscy_utils.log_images.detach_sample so image-grid logging preserves all channels for each view (instead of using the first view’s channel count for all views), preventing silent channel drops in virtual staining logs.

Changes:

  • Update detach_sample to iterate over each view’s own channel count when building per-sample grid rows.
  • Add a regression test covering mixed channel counts across views (e.g., source=1ch, target/pred=2ch) and validate the rendered grid shape.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/viscy-utils/src/viscy_utils/log_images.py Fix per-view channel iteration so multi-channel views aren’t truncated during logging.
packages/viscy-utils/src/viscy_utils/log_images_test.py Adds regression tests for mixed-channel view logging and rendered grid sizing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/viscy-utils/tests/test_log_images.py
The regression test was under src/viscy_utils/, which the repo-wide
pytest testpaths (packages/*/tests, applications/*/tests) does not
collect, so it would not run in CI. Move it to
packages/viscy-utils/tests/test_log_images.py.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@srivarra srivarra merged commit 3d179c6 into main Jun 5, 2026
30 checks passed
@edyoshikun edyoshikun deleted the fix/log-images-multichannel branch June 5, 2026 12:45
ieivanov added a commit that referenced this pull request Jun 12, 2026
Resolve conflicts from main's monorepo re-staging:
- Keep branch's unified rotation_tta_transforms TTA in cytoland engine;
  drop main's superseded square-padding helpers, graft in main's n<1 guard.
- Take main elsewhere: _read_norm_meta rename (+public alias), qc->viscy-qc
  package rename, log_images multi-channel-view fix (#457), docs/READMEs,
  moved svideo_1.png. Regenerate uv.lock.

Co-Authored-By: Claude Opus 4.8 (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.

3 participants