Skip to content

feat: memory-optimized metric computation with streaming - #6

Merged
Kirscher merged 3 commits into
mainfrom
feat/memory-optimized-compute
Feb 13, 2026
Merged

Kirscher merged 3 commits into
mainfrom
feat/memory-optimized-compute

Conversation

@Kirscher

Copy link
Copy Markdown
Owner
  • Eliminate PASS 2: track per-class max probs during single fold pass
  • Stream folds one at a time, free immediately after accumulation
  • Streaming BA-ECE: process raters individually instead of stacking full distance/band arrays (avoids O(n_raters * volume) peak memory)
  • ACE: concatenate per-rater results instead of np.repeat tiling
  • Vectorized majority consensus (replaces slow np.apply_along_axis)
  • Use int8 for GT labels, uint8 for prediction labels
  • Force glibc memory return via malloc_trim after each case
  • Fix GT float-to-int truncation: np.round before astype(int)
  • Add per-metric timing profiler with CSV output
  • Add --gt-label-offset CLI param for label remapping
  • Add AURC per-case CSV export for bootstrap resampling
  • Add global-metric bootstrap (bootstrap_aurc_file)

- Eliminate PASS 2: track per-class max probs during single fold pass
- Stream folds one at a time, free immediately after accumulation
- Streaming BA-ECE: process raters individually instead of stacking
  full distance/band arrays (avoids O(n_raters * volume) peak memory)
- ACE: concatenate per-rater results instead of np.repeat tiling
- Vectorized majority consensus (replaces slow np.apply_along_axis)
- Use int8 for GT labels, uint8 for prediction labels
- Force glibc memory return via malloc_trim after each case
- Fix GT float-to-int truncation: np.round before astype(int)
- Add per-metric timing profiler with CSV output
- Add --gt-label-offset CLI param for label remapping
- Add AURC per-case CSV export for bootstrap resampling
- Add global-metric bootstrap (bootstrap_aurc_file)
Copilot AI review requested due to automatic review settings February 10, 2026 08:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors ensemble-metric computation to reduce peak memory usage by streaming fold data (single-pass accumulation) and introduces new outputs/utilities for profiling and bootstrapping AURC.

Changes:

  • Stream fold processing to avoid stacking all folds in memory; add precomputed intermediate artifacts reused by metrics.
  • Add streaming BA-ECE implementation and a more memory-efficient GED implementation.
  • Add per-case AURC export for bootstrap resampling + implement AURC-specific bootstrapping; add timing profiler CSV and GT label offset CLI flag.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
src/ensemble_metrics/utils.py Optimizes majority consensus and adjusts GT loading/casting; adds STAPLE consensus computation fallback.
src/ensemble_metrics/metrics.py Updates metric implementations to consume precomputed artifacts and gate map saving via --save-maps; adds AURC per-case export.
src/ensemble_metrics/metric_functions.py Adds iterative GED computation and introduces compute_ba_ece_streaming to reduce BA-ECE peak memory.
src/ensemble_metrics/compute.py Implements streaming fold pass, memory release hooks, timing logging, --save-maps, and --gt-label-offset.
src/ensemble_metrics/bootstrap.py Adds global-metric bootstrap for AURC using per-case data and integrates it into directory bootstrapping and method comparison.

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

Comment on lines +104 to +109
best_count = np.zeros(raters[0].shape, dtype=np.uint8)
winner = np.zeros(raters[0].shape, dtype=np.int8)
for c in range(num_classes):
count = np.zeros(raters[0].shape, dtype=np.uint8)
for r in raters:
count += (r == c).view(np.uint8)

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

calculate_majority_consensus stores the winning label in an np.int8 array and vote counts in np.uint8. This will overflow if labels exceed 127 or if the number of raters exceeds 255, producing incorrect consensus labels/counts. Use a wider integer dtype (e.g., int16/int32 for labels and uint16/int16 for counts) or pick dtypes based on num_classes/len(raters).

Suggested change
best_count = np.zeros(raters[0].shape, dtype=np.uint8)
winner = np.zeros(raters[0].shape, dtype=np.int8)
for c in range(num_classes):
count = np.zeros(raters[0].shape, dtype=np.uint8)
for r in raters:
count += (r == c).view(np.uint8)
best_count = np.zeros(raters[0].shape, dtype=np.uint16)
winner = np.zeros(raters[0].shape, dtype=np.int32)
for c in range(num_classes):
count = np.zeros(raters[0].shape, dtype=np.uint16)
for r in raters:
count += (r == c).astype(np.uint16)

Copilot uses AI. Check for mistakes.
Comment on lines 154 to 158
img = nib.load(gt_path)
data = img.get_fdata().astype(np.int32)
data = np.round(img.get_fdata()).astype(np.int8)
gt = {
"raters": np.expand_dims(data, axis=0),
"consensus": data,

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

Ground-truth labels are now rounded and cast to np.int8. If the NIfTI contains labels outside [-128, 127] (or if later remapping pushes values higher), this will wrap/overflow and silently corrupt labels. Consider keeping int16/int32 for GT label storage (or validate the label range before downcasting).

Copilot uses AI. Check for mistakes.
Comment on lines 270 to +277
metric.compute_case(
case_id=case_id,
preds_per_fold=preds_per_fold,
preds_per_fold=None,
gt=gt,
affine=affine,
case_output_dir=case_output_dir
case_output_dir=case_output_dir,
save_maps=args.save_maps,
precomputed=precomputed

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

compute.py calls metric.compute_case(preds_per_fold=None, ..., precomputed=precomputed), but the compute_case signatures still type preds_per_fold as Dict[int, np.ndarray] and metrics access precomputed[...] unconditionally. This is an API inconsistency that can lead to runtime failures for any caller that still passes a real preds_per_fold without precomputed (or relies on type checking). Consider making preds_per_fold optional everywhere (and enforcing/validating precomputed when the streaming path is used), or keep passing a minimal preds_per_fold structure.

Copilot uses AI. Check for mistakes.
band_dist_sum = np.zeros(n_bands, dtype=np.float64)
band_count = np.zeros(n_bands, dtype=np.int64)

conf_flat = confidence.ravel().astype(np.float32) # (V,)

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

compute_ba_ece_streaming does confidence.ravel().astype(np.float32), which forces a full copy of the confidence map even if it is already float32. For large volumes this can be a significant extra allocation. Prefer np.asarray(confidence, dtype=np.float32).ravel() (or astype(np.float32, copy=False) in newer NumPy) to avoid unnecessary copies.

Suggested change
conf_flat = confidence.ravel().astype(np.float32) # (V,)
conf_flat = np.asarray(confidence, dtype=np.float32).ravel() # (V,)

Copilot uses AI. Check for mistakes.
# Try to find matching confid column
if suffix == "overall_risk":
cc = "confid_overall_confid"
label = "overall_aurc"

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

In bootstrap_aurc_file, the overall AURC label is set to overall_aurc, which then becomes metric_name = 'aurc_overall_aurc'. This looks inconsistent with the rest of the code (e.g., _compare_aurc_per_case uses aurc_overall) and makes the output harder to consume programmatically. Consider using a consistent suffix like overall so the final name is aurc_overall.

Suggested change
label = "overall_aurc"
label = "overall"

Copilot uses AI. Check for mistakes.

if first_shape is None:
first_shape = pred.shape[1:]
num_classes = pred.shape[0]

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

This assignment to 'num_classes' is unnecessary as it is redefined before this value is used.

Copilot uses AI. Check for mistakes.
gc.collect()
try:
ctypes.CDLL("libc.so.6").malloc_trim(0)
except (OSError, AttributeError):

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change
except (OSError, AttributeError):
except (OSError, AttributeError):
# Best-effort optimization: ignore if libc or malloc_trim is unavailable.

Copilot uses AI. Check for mistakes.
try:
rss_kb = int(open("/proc/self/statm").read().split()[1]) * (os.sysconf("SC_PAGE_SIZE") // 1024)
tqdm.write(f" RSS after cleanup: {rss_kb/1024:.0f} MB")
except Exception:

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change
except Exception:
except Exception:
# Best-effort logging only: ignore if /proc/self/statm or sysconf are unavailable or unreadable.

Copilot uses AI. Check for mistakes.
gc.collect()
try:
ctypes.CDLL("libc.so.6").malloc_trim(0)
except (OSError, AttributeError):

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change
except (OSError, AttributeError):
except (OSError, AttributeError):
# Best-effort memory trimming; ignore if libc/malloc_trim is unavailable or fails.

Copilot uses AI. Check for mistakes.

# Log RSS so we can watch for leaks
try:
rss_kb = int(open("/proc/self/statm").read().split()[1]) * (os.sysconf("SC_PAGE_SIZE") // 1024)

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

File is opened but is not closed.

Suggested change
rss_kb = int(open("/proc/self/statm").read().split()[1]) * (os.sysconf("SC_PAGE_SIZE") // 1024)
with open("/proc/self/statm") as f:
rss_kb = int(f.read().split()[1]) * (os.sysconf("SC_PAGE_SIZE") // 1024)

Copilot uses AI. Check for mistakes.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0b15909f0

ℹ️ 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 os.path.exists(gt_path):
img = nib.load(gt_path)
data = img.get_fdata().astype(np.int32)
data = np.round(img.get_fdata()).astype(np.int8)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Use non-lossy dtype when loading ground-truth labels

Casting NIfTI labels to np.int8 will wrap any class value above 127 (for example, common 0/255 masks become 0/-1), which silently corrupts class IDs before consensus and downstream metrics are computed. This is a regression from the previous int32 behavior and will produce incorrect Dice/GED/AURC results on datasets that use high-valued or non-consecutive label encodings.

Useful? React with 👍 / 👎.

Comment thread src/ensemble_metrics/bootstrap.py Outdated
Comment on lines +741 to +742
shifted = boot_diffs - np.mean(boot_diffs)
p_value = max(np.mean(np.abs(shifted) >= np.abs(observed_diff)), 1.0 / n_bootstrap)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Center AURC bootstrap test on observed difference

The two-sided p-value is computed after centering by np.mean(boot_diffs) instead of the observed statistic, which mis-specifies the null distribution when the bootstrap estimator is biased (a common case for nonlinear statistics like AURC with limited sample sizes). That can inflate or deflate significance decisions; the module’s own paired_bootstrap_test correctly centers with observed_diff.

Useful? React with 👍 / 👎.

Comment on lines +136 to +137
mean_dice = {k: v / n_pairs for k, v in accum.items()}
mean_dice["overall_dice"] = float(np.mean(list(mean_dice.values())))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep GED aggregation consistent with prior definition

This computes class Dice as an unweighted mean across pairwise Dice values, which is not equivalent to the previous global intersection/union aggregation used by compute_ged. When foreground volume varies across rater/fold pairs, the GED value shifts materially, so this memory refactor changes metric semantics and breaks comparability with earlier experiment outputs.

Useful? React with 👍 / 👎.

- Refactor bootstrap.py to route multicolumn (GED, dice_vs_gt),
  multirow (pairwise_dice), and AURC metric files correctly
- Add _SKIP_FILES, _MULTICOLUMN_FILES, _MULTIROW_FILES routing
- Fix p-value centering bug (use observed_diff, not mean(boot_diffs))

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 9 comments.


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

Comment on lines +15 to +22

def _release_memory():
"""Force Python GC and glibc to return freed pages to the OS."""
gc.collect()
try:
ctypes.CDLL("libc.so.6").malloc_trim(0)
except (OSError, AttributeError):
pass

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

_release_memory() loads libc.so.6 via ctypes.CDLL(...) on every call. Since this is invoked inside the fold loop, repeatedly loading the shared library can add noticeable overhead; consider caching the loaded libc handle once at module scope (and only calling malloc_trim when available) to reduce per-iteration cost.

Suggested change
def _release_memory():
"""Force Python GC and glibc to return freed pages to the OS."""
gc.collect()
try:
ctypes.CDLL("libc.so.6").malloc_trim(0)
except (OSError, AttributeError):
pass
# Cache libc's malloc_trim (if available) at module load time to avoid
# repeatedly loading the shared library in tight loops.
try:
_LIBC = ctypes.CDLL("libc.so.6")
_MALLOC_TRIM = getattr(_LIBC, "malloc_trim", None)
except OSError:
_LIBC = None
_MALLOC_TRIM = None
def _release_memory():
"""Force Python GC and glibc to return freed pages to the OS."""
gc.collect()
# Only call malloc_trim if it is available on this platform.
if _MALLOC_TRIM is not None:
try:
_MALLOC_TRIM(0)
except AttributeError:
# In case the symbol disappears or is unusable at runtime.
pass

Copilot uses AI. Check for mistakes.
Comment on lines +463 to +467
gc.collect()
try:
ctypes.CDLL("libc.so.6").malloc_trim(0)
except (OSError, AttributeError):
pass

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

ctypes.CDLL("libc.so.6") is called inside the per-rater loop, which can be unnecessarily expensive. Consider caching the libc handle once (or using the shared _release_memory() helper) so malloc_trim doesn't require re-loading the shared library for every rater.

Copilot uses AI. Check for mistakes.
Comment on lines +361 to +377
def bootstrap_aurc_file(
filepath: str,
n_bootstrap: int = 10000,
ci_level: float = 0.95,
method: str = "bca",
random_state: Optional[int] = None
) -> List[BootstrapResult]:
"""
Bootstrap AURC by resampling cases and recomputing the global metric.

The file must be an ``aurc_per_case.csv`` containing per-case risk and
confidence columns (produced by ``AURCMetric.export_summaries``).

Parameters
----------
filepath : str
Path to ``aurc_per_case.csv``.

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

New bootstrap routing/handlers (bootstrap_aurc_file, multi-column, and multi-row support) aren't covered by existing pytest cases (no tests reference aurc_per_case.csv, pairwise_dice_per_case.csv, or multi-column per-case CSVs). Adding targeted tests with small synthetic CSVs would help prevent regressions in CI and validate that file routing + column pairing behaves as intended.

Copilot uses AI. Check for mistakes.
Comment on lines +409 to +414
def compute_ba_ece_streaming(
confidence: np.ndarray,
labels: np.ndarray,
pred_labels: np.ndarray,
edges: Union[List[float], Tuple[float, ...]] = (0, 3, 7, 15, np.inf),
) -> Dict[str, object]:

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

compute_ba_ece_streaming introduces a new code path intended to be numerically equivalent to compute_ba_ece while reducing memory. Since tests/test_metric_functions.py already covers other helpers in this module, consider adding a unit test that compares compute_ba_ece_streaming(...) vs compute_ba_ece(...) on a small synthetic example to lock in correctness.

Copilot uses AI. Check for mistakes.
Comment on lines 24 to 33
def compute_case(
self,
case_id: str,
preds_per_fold: Dict[int, np.ndarray],
gt: Optional[np.ndarray] = None,
affine: Optional[np.ndarray] = None,
case_output_dir: Optional[str] = None
case_output_dir: Optional[str] = None,
save_maps: bool = False,
precomputed: Optional[Dict[str, Any]] = None
) -> Dict[str, Any]:

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

compute_case now effectively requires precomputed (all metric implementations index into precomputed[...]), but the signature still requires preds_per_fold: Dict[int, np.ndarray] and doesn't validate inputs. Consider changing preds_per_fold to Optional[Dict[int, np.ndarray]] = None and adding a clear ValueError when neither precomputed nor preds_per_fold is provided (or keep a fallback path that computes needed intermediates from preds_per_fold).

Copilot uses AI. Check for mistakes.
Comment on lines 270 to 278
metric.compute_case(
case_id=case_id,
preds_per_fold=preds_per_fold,
preds_per_fold=None,
gt=gt,
affine=affine,
case_output_dir=case_output_dir
case_output_dir=case_output_dir,
save_maps=args.save_maps,
precomputed=precomputed
)

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

metric.compute_case(... preds_per_fold=None ...) passes None into a parameter typed/expected as Dict[int, np.ndarray]. Even if current metrics ignore preds_per_fold, this is a footgun for future metrics and for type checking; align by making preds_per_fold optional (or omit it entirely) and validate that precomputed is provided when using the streaming path.

Copilot uses AI. Check for mistakes.
off = args.gt_label_offset
for key in ("raters", "consensus"):
arr = gt[key]
gt[key] = np.where(arr > 0, arr + off, arr)

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

np.where(arr > 0, arr + off, arr) will upcast to a larger integer dtype (typically int64) when off is a Python int, which can significantly increase peak memory for large volumes and partially defeats the switch to int8 GT labels. Consider performing the offset in a controlled dtype (e.g., cast to int16/int32 first, add off, then cast back to the intended GT dtype).

Suggested change
gt[key] = np.where(arr > 0, arr + off, arr)
# Perform offset in a controlled integer dtype to avoid upcasting to int64
orig_dtype = arr.dtype
work = arr.astype(np.int32, copy=False)
off32 = np.int32(off)
tmp = np.where(work > 0, work + off32, work)
gt[key] = tmp.astype(orig_dtype, copy=False)

Copilot uses AI. Check for mistakes.
Comment on lines +99 to +105
Uses per-class vote counting instead of np.apply_along_axis for
O(num_classes) memory instead of O(num_raters * volume).
"""
num_classes = max(int(r.max()) for r in raters) + 1
# Vote counting: for each class, count how many raters agree
best_count = np.zeros(raters[0].shape, dtype=np.uint8)
winner = np.zeros(raters[0].shape, dtype=np.int8)

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

The docstring claims this approach uses "O(num_classes) memory", but the implementation allocates best_count/winner arrays of shape volume (and a temporary count array of the same size per class). Consider updating the wording to reflect the actual memory behavior (reduces from O(num_raters * volume) to ~O(volume)).

Suggested change
Uses per-class vote counting instead of np.apply_along_axis for
O(num_classes) memory instead of O(num_raters * volume).
"""
num_classes = max(int(r.max()) for r in raters) + 1
# Vote counting: for each class, count how many raters agree
best_count = np.zeros(raters[0].shape, dtype=np.uint8)
winner = np.zeros(raters[0].shape, dtype=np.int8)
Uses per-class vote counting instead of np.apply_along_axis to
reduce memory usage from O(num_raters * volume) to approximately
O(volume), independent of the number of raters.
"""
num_classes = max(int(r.max()) for r in raters) + 1
# Vote counting: for each class, count how many raters agree
best_count = np.zeros(raters[0].shape, dtype=np.uint8)

Copilot uses AI. Check for mistakes.
Comment on lines +422 to +424
def _aurc_from_indices(idx):
return _compute_aurc(risks_all[idx], confids_all[idx])

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

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

Variable _aurc_from_indices is not used.

Suggested change
def _aurc_from_indices(idx):
return _compute_aurc(risks_all[idx], confids_all[idx])

Copilot uses AI. Check for mistakes.
The suffix extraction used .replace('risk_', '') which replaced all
occurrences, turning 'risk_overall_risk_a' into 'overall__a' instead
of 'overall_risk_a'. Use .replace('risk_', '', 1).removesuffix('_a')
to correctly extract the suffix.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 10 comments.


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

"expected_entropy_map": expected_entropy_map,
"max_conf": max_conf,
}
del mean_probs, expected_entropy_map, max_conf, consensus_seg

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

The variables are deleted from the local namespace on line 264 but still referenced in the precomputed dictionary on line 257-263. This is intentional to free the local references while keeping the data accessible through the dictionary, but the deletion is misleading and provides no memory benefit since the objects are still referenced by the dictionary. Consider removing these deletions or adding a comment explaining that this is intentional to clarify the reference is only through the dictionary.

Suggested change
del mean_probs, expected_entropy_map, max_conf, consensus_seg

Copilot uses AI. Check for mistakes.

# Background = 1 - sum(foreground probs)
class_probs[0] = np.clip(1.0 - class_probs[1:].sum(axis=0), 0, 1)
consensus = np.argmax(class_probs, axis=0).astype(np.int8)

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

The consensus returned from _compute_staple uses np.int8, which can overflow if num_classes exceeds 128, just like in calculate_majority_consensus. This should use a larger dtype or validate the number of classes.

Suggested change
consensus = np.argmax(class_probs, axis=0).astype(np.int8)
consensus = np.argmax(class_probs, axis=0).astype(np.int16)

Copilot uses AI. Check for mistakes.
Comment on lines +16 to +22
def _release_memory():
"""Force Python GC and glibc to return freed pages to the OS."""
gc.collect()
try:
ctypes.CDLL("libc.so.6").malloc_trim(0)
except (OSError, AttributeError):
pass

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

The malloc_trim call with CDLL("libc.so.6") is Linux-specific and will fail on non-Linux systems. While the try-except handles this gracefully, the error message could be confusing. Consider either documenting this as a Linux-only optimization or using a more specific exception check (e.g., checking platform first).

Copilot uses AI. Check for mistakes.
num_classes = max(int(r.max()) for r in raters) + 1
# Vote counting: for each class, count how many raters agree
best_count = np.zeros(raters[0].shape, dtype=np.uint8)
winner = np.zeros(raters[0].shape, dtype=np.int8)

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

The winner array uses np.int8, which can only represent values from -128 to 127. If num_classes exceeds 128, this will cause overflow. Since this is a segmentation task and medical imaging datasets can have many classes, consider using np.uint8 (0-255 range) or a larger dtype to handle more classes safely.

Suggested change
winner = np.zeros(raters[0].shape, dtype=np.int8)
winner = np.zeros(raters[0].shape, dtype=np.uint16)

Copilot uses AI. Check for mistakes.
if os.path.exists(gt_path):
img = nib.load(gt_path)
data = img.get_fdata().astype(np.int32)
data = np.round(img.get_fdata()).astype(np.int8)

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

Using np.int8 for ground truth labels has the same overflow issue as in the consensus calculation. Labels with values greater than 127 will overflow. Since this is loading actual ground truth data which may have arbitrary label values, consider using np.int16 or np.int32 for safety, or at least add a check to ensure labels fit within int8 range.

Copilot uses AI. Check for mistakes.
expected_entropy_map /= num_folds
del expected_entropy_sum

consensus_seg = np.argmax(mean_probs, axis=0).astype(np.uint8)

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

The consensus_seg uses np.uint8, which limits the number of classes to 256 (0-255). If num_classes exceeds 256, np.argmax can return values greater than 255, causing overflow when cast to uint8. This should either validate that num_classes fits within uint8 range or use a larger dtype like uint16.

Suggested change
consensus_seg = np.argmax(mean_probs, axis=0).astype(np.uint8)
# Use a label dtype that can represent all classes without overflow.
# Keep uint8 when safe (<= 256 classes) to preserve existing behavior.
num_classes = mean_probs.shape[0]
label_dtype = np.uint8 if num_classes <= 256 else np.uint16
consensus_seg = np.argmax(mean_probs, axis=0).astype(label_dtype)

Copilot uses AI. Check for mistakes.
Comment on lines +199 to +200
# Store argmax labels (uint8 saves memory)
labels_per_fold[fold_idx] = np.argmax(pred, axis=0).astype(np.uint8)

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

The labels_per_fold also uses np.uint8, which has the same overflow issue as consensus_seg. If num_classes exceeds 256, np.argmax can return values that overflow when cast to uint8. This is consistent throughout the code but should be validated or use a larger dtype.

Suggested change
# Store argmax labels (uint8 saves memory)
labels_per_fold[fold_idx] = np.argmax(pred, axis=0).astype(np.uint8)
# Store argmax labels; use smallest dtype that can represent all classes
label_dtype = np.uint8 if num_classes <= np.iinfo(np.uint8).max else np.uint16
labels_per_fold[fold_idx] = np.argmax(pred, axis=0).astype(label_dtype)

Copilot uses AI. Check for mistakes.
Comment on lines +294 to +298
# Log RSS so we can watch for leaks
try:
rss_kb = int(open("/proc/self/statm").read().split()[1]) * (os.sysconf("SC_PAGE_SIZE") // 1024)
tqdm.write(f" RSS after cleanup: {rss_kb/1024:.0f} MB")
except Exception:

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

Reading /proc/self/statm for RSS measurement is Linux-specific and will fail silently on non-Linux systems (Windows, macOS). While the try-except handles this gracefully, consider using psutil.Process().memory_info().rss for cross-platform compatibility if the library is available, or add a comment indicating this is a Linux-only debugging feature.

Suggested change
# Log RSS so we can watch for leaks
try:
rss_kb = int(open("/proc/self/statm").read().split()[1]) * (os.sysconf("SC_PAGE_SIZE") // 1024)
tqdm.write(f" RSS after cleanup: {rss_kb/1024:.0f} MB")
except Exception:
# Log RSS so we can watch for leaks (best-effort, cross-platform when psutil is available)
try:
try:
import psutil
process = psutil.Process()
rss_kb = process.memory_info().rss // 1024
except Exception:
# Fallback: Linux-specific /proc-based RSS measurement
rss_kb = int(open("/proc/self/statm").read().split()[1]) * (
os.sysconf("SC_PAGE_SIZE") // 1024
)
tqdm.write(f" RSS after cleanup: {rss_kb/1024:.0f} MB")
except Exception:
# RSS logging is a debugging aid; ignore any failures (e.g., on unsupported platforms)

Copilot uses AI. Check for mistakes.

Uses per-class vote counting instead of np.apply_along_axis for
O(num_classes) memory instead of O(num_raters * volume).
"""

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

The function assumes raters is a non-empty list and accesses raters[0].shape without validation. If raters is an empty list, this will raise an IndexError. Consider adding a check for empty input or document that the input must be non-empty.

Suggested change
"""
"""
if not raters:
raise ValueError("calculate_majority_consensus expected a non-empty list of rater arrays.")

Copilot uses AI. Check for mistakes.
for c in range(num_classes):
count = np.zeros(raters[0].shape, dtype=np.uint8)
for r in raters:
count += (r == c).view(np.uint8)

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

The .view(np.uint8) call is unsafe here. When converting a boolean array to uint8, you should use .astype(np.uint8) instead. The .view() method reinterprets the memory without conversion, which can lead to incorrect results depending on how boolean values are stored in memory (they may not be exactly 0 and 1 bytes).

Suggested change
count += (r == c).view(np.uint8)
count += (r == c).astype(np.uint8)

Copilot uses AI. Check for mistakes.
@Kirscher

Copy link
Copy Markdown
Owner Author

I merge the PR
Issues raised by Copilot are corrected for most of them
For unint8 overflow no problem since we have no dataset with 127+ classes

@Kirscher
Kirscher merged commit 4e309b9 into main Feb 13, 2026
7 checks passed
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.

2 participants