fix: 2D images inference support + consensus type for GA - #7
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adaeca5d4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if data.ndim == 4 and data.shape[1] == 1: | ||
| # 2D: squeeze dummy depth → (C, H, W) | ||
| data = data.squeeze(1) |
There was a problem hiding this comment.
Distinguish 2D from single-slice 3D before squeezing depth
The new data.shape[1] == 1 branch treats every (C, 1, H, W) tensor as 2D, but valid 3D cases with depth D=1 have the same shape. In that scenario this drops a real spatial axis ((C, H, W)), and downstream GT from NIfTI remains 3D, so comparisons like Dice can fail on shape mismatch or silently evaluate the wrong geometry. Please gate this on actual image dimensionality (e.g., metadata/reference NIfTI) rather than D==1 alone.
Useful? React with 👍 / 👎.
| while pred.ndim > 2 and pred.shape[-1] == 1: | ||
| pred = pred.squeeze(-1) |
There was a problem hiding this comment.
Keep singleton depth axis for true 3D predictions
The unconditional trailing-axis squeeze removes any final size-1 dimension for all predictions, which collapses legitimate 3D volumes with depth 1 into 2D arrays. That changes tensor rank before one-hot/argmax and can break GT-aligned metrics for single-slice volumes. This should only strip the known synthetic axis from 2D exports, not every trailing singleton dimension.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR adds support for 2D image inference and introduces "consensus" as a new consensus type for ground truth loading. The changes address a bug where 2D images from nnUNet (stored as (C, 1, H, W)) were not being handled correctly.
Changes:
- Added logic to detect and handle 2D images by squeezing the dummy depth dimension in
load_prediction - Added dimension cleanup logic in
standardize_predictionto handle trailing singleton dimensions - Added "consensus" as a valid consensus type option in the CLI validation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/ensemble_metrics/utils.py | Added 2D image detection in load_prediction to squeeze dummy depth dimension; Added while loop in standardize_prediction to handle trailing singleton dimensions |
| src/ensemble_metrics/compute.py | Updated consensus type validation to include "consensus" as a valid option |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if args.consensus_type not in ["staple", "majority", "consensus", "none"]: | ||
| raise ValueError("consensus-type must be one of 'staple', 'majority', 'consensus', or 'none'") |
There was a problem hiding this comment.
The validation accepts "consensus" and "majority" as consensus types, but the load_ground_truth function in utils.py (lines 186-194) only handles "staple" by computing it when the file is missing. For "consensus" and "majority", if the corresponding file doesn't exist, it will raise a FileNotFoundError instead of computing the consensus. This inconsistency will cause runtime errors when users specify these types without providing pre-computed consensus files.
| if args.consensus_type not in ["staple", "majority", "consensus", "none"]: | |
| raise ValueError("consensus-type must be one of 'staple', 'majority', 'consensus', or 'none'") | |
| if args.consensus_type not in ["staple", "majority", "none"]: | |
| raise ValueError("consensus-type must be one of 'staple', 'majority', or 'none'") |
No description provided.