Skip to content

Experiment/hybrid mil eval - #32

Merged
ejdam87 merged 259 commits into
masterfrom
experiment/hybrid-mil-eval
Aug 3, 2026
Merged

Experiment/hybrid mil eval#32
ejdam87 merged 259 commits into
masterfrom
experiment/hybrid-mil-eval

Conversation

@ejdam87

@ejdam87 ejdam87 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

This PR adds MIL-based callbacks and evaluations.

Summary by CodeRabbit

  • New Features
    • Added MIL-focused evaluation and prediction workflows for tile- and slide-level results.
    • Added curve analysis, nested metrics, prediction reports, and histogram outputs across supported workflows.
    • Added new threshold-estimation and test-suite configurations for PGP and Virchow2 models.
  • Enhancements
    • Updated supported experiments to use hybrid MIL models.
    • Improved prediction outputs and artifact generation.
    • Refined default tile and slide thresholds for improved evaluation consistency.

@ejdam87
ejdam87 requested review from a team, Adames4 and vejtek July 27, 2026 06:36
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds structured MIL test outputs, specialized curve, histogram, metric, and prediction callbacks, and Hydra configurations for hybrid PGP and Virchow2 training, testing, prediction, and threshold estimation.

Changes

MIL evaluation callbacks

Layer / File(s) Summary
Model and callback contracts
prostate_cancer/attention_mil_model.py, prostate_cancer/callbacks/*base.py, prostate_cancer/callbacks/__init__.py
test_step now returns MILModelOutput. Shared callback bases centralize curve, histogram, and nested-metric processing for test and prediction epochs.
Specialized MIL and prediction callbacks
prostate_cancer/callbacks/curves_callback_*.py, prostate_cancer/callbacks/nested_metrics_callback_mil.py, prostate_cancer/callbacks/*histograms_callback_*.py, prostate_cancer/callbacks/mil_prediction_callback.py
Specialized callbacks collect masked tile outputs, slide predictions, metadata-derived labels, nested metric keys, and MLflow prediction artifacts.
Callback configuration wiring
configs/callbacks/*
Hydra callback configurations select MIL, slide, and tile callback implementations and connect thresholds through interpolations.

Hybrid model experiment configuration

Layer / File(s) Summary
Hybrid model experiments and thresholds
configs/model/*, configs/model/checkpoints/*, configs/experiment/train/*, configs/experiment/predict/*, configs/experiment/test/*
Training, prediction, testing, and threshold-estimation configurations select hybrid PGP and Virchow2 models, datasets, callbacks, and calibrated threshold values.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: vejtek

Sequence Diagram(s)

sequenceDiagram
  participant Trainer
  participant ProstateCancerAttentionMIL
  participant EvaluationCallbacks
  participant MLflow
  Trainer->>ProstateCancerAttentionMIL: run test or prediction batch
  ProstateCancerAttentionMIL-->>Trainer: return MILModelOutput
  Trainer->>EvaluationCallbacks: invoke batch-end hooks
  EvaluationCallbacks->>EvaluationCallbacks: collect masked outputs and metadata
  Trainer->>EvaluationCallbacks: invoke epoch-end hooks
  EvaluationCallbacks->>MLflow: log curves, histograms, metrics, or predictions
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the pull request's hybrid MIL evaluation changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch experiment/hybrid-mil-eval

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
prostate_cancer/callbacks/curves_callback_base.py (1)

111-127: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Skip plotting when buffers are empty.

on_test_epoch_end() and on_predict_epoch_end() run on the base classes even when no batch callbacks added data.

  • prostate_cancer/callbacks/curves_callback_base.py#L111-L127: return before torch.cat(...) if self.preds or self.targets is empty.
  • prostate_cancer/callbacks/tile_histograms_callback_base.py#L19-L60: return before np.concatenate(...) if self.all_preds or self.all_labels is empty.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@prostate_cancer/callbacks/curves_callback_base.py` around lines 111 - 127,
Guard _plot_and_clear in prostate_cancer/callbacks/curves_callback_base.py
(lines 111-127) by returning before torch.cat when either self.preds or
self.targets is empty. Apply the same early-return guard in the plotting method
in prostate_cancer/callbacks/tile_histograms_callback_base.py (lines 19-60)
before np.concatenate when either self.all_preds or self.all_labels is empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@configs/model/pgp_hybrid_mil_model.yaml`:
- Around line 3-5: Bind the required lr, tl_threshold, and sl_threshold
parameters for ProstateCancerAttentionMIL using the shared experiment/default
configuration or explicit valid overrides. Apply the same change to
configs/model/pgp_hybrid_mil_model.yaml lines 3-5 and
configs/model/virchow2_hybrid_mil_model.yaml lines 3-5, removing the mandatory
??? placeholders so Hydra can instantiate both models.

In `@prostate_cancer/callbacks/curves_callback_base.py`:
- Line 13: Update CurvesCallbackBase so the ROC and precision-recall artifact
names generated in its plotting methods include the callback level instead of
fixed tile-level names. Also update TileHistogramsCallbackBase to include its
slide/tile level in the histograms artifact name, or move that naming into
subclasses, ensuring artifacts do not collide in the shared MLflow plots path.
Apply the changes in prostate_cancer/callbacks/curves_callback_base.py (anchor,
line 13) and prostate_cancer/callbacks/tile_histograms_callback_base.py
(sibling, lines 12-19).

In `@prostate_cancer/callbacks/nested_metrics_callback_mil.py`:
- Around line 26-31: Update the keys construction in the nested metrics callback
to call nonzero() once on mask_bool, then use the resulting selected row indices
to assemble slide names from metadata. Remove the nested per-tile iteration and
scalar if condition so GPU synchronization occurs only during the single index
materialization.

---

Outside diff comments:
In `@prostate_cancer/callbacks/curves_callback_base.py`:
- Around line 111-127: Guard _plot_and_clear in
prostate_cancer/callbacks/curves_callback_base.py (lines 111-127) by returning
before torch.cat when either self.preds or self.targets is empty. Apply the same
early-return guard in the plotting method in
prostate_cancer/callbacks/tile_histograms_callback_base.py (lines 19-60) before
np.concatenate when either self.all_preds or self.all_labels is empty.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 03830ea8-6940-46ae-ba68-6685b68d0e18

📥 Commits

Reviewing files that changed from the base of the PR and between 3a07ef2 and ae45736.

📒 Files selected for processing (49)
  • configs/callbacks/combined_test_suite_mil.yaml
  • configs/callbacks/curves.yaml
  • configs/callbacks/curves_mil.yaml
  • configs/callbacks/curves_sl.yaml
  • configs/callbacks/nested_metrics_mil.yaml
  • configs/callbacks/slide_histograms_mil.yaml
  • configs/callbacks/slide_threshold_mil.yaml
  • configs/callbacks/tile_histograms.yaml
  • configs/callbacks/tile_histograms_mil.yaml
  • configs/callbacks/tile_threshold_mil.yaml
  • configs/experiment/predict/bag_of_embeddings/pgp/base.yaml
  • configs/experiment/predict/bag_of_embeddings/pgp/mil_predictions/base.yaml
  • configs/experiment/predict/bag_of_embeddings/pgp/mil_predictions/mmci_sl_group_1.yaml
  • configs/experiment/predict/bag_of_embeddings/pgp/mil_predictions/mmci_tl_val.yaml
  • configs/experiment/predict/bag_of_embeddings/pgp/slide_threshold.yaml
  • configs/experiment/predict/bag_of_embeddings/virchow2/base.yaml
  • configs/experiment/predict/bag_of_embeddings/virchow2/mil_predictions/mmci_sl_group_1.yaml
  • configs/experiment/predict/bag_of_embeddings/virchow2/slide_threshold.yaml
  • configs/experiment/test/bag_of_embeddings/base.yaml
  • configs/experiment/test/bag_of_embeddings/pgp/base.yaml
  • configs/experiment/test/bag_of_embeddings/pgp/test_suite/base.yaml
  • configs/experiment/test/bag_of_embeddings/pgp/test_suite/mmci_tl_test.yaml
  • configs/experiment/test/bag_of_embeddings/pgp/tile_threshold.yaml
  • configs/experiment/test/bag_of_embeddings/virchow2/base.yaml
  • configs/experiment/test/bag_of_embeddings/virchow2/test_suite/base.yaml
  • configs/experiment/test/bag_of_embeddings/virchow2/test_suite/mmci_tl_test.yaml
  • configs/experiment/test/bag_of_embeddings/virchow2/tile_threshold.yaml
  • configs/experiment/train/bag_of_embeddings/pgp.yaml
  • configs/experiment/train/bag_of_embeddings/virchow2.yaml
  • configs/model/checkpoints/mmci_tl/pgp_hybrid_mil.yaml
  • configs/model/checkpoints/mmci_tl/virchow2_hybrid_mil.yaml
  • configs/model/pgp_hybrid_mil_model.yaml
  • configs/model/virchow2_hybrid_mil_model.yaml
  • preprocessing/embeddings/tile_embeddings.py
  • preprocessing/masks/quality_control_v2.py
  • prostate_cancer/attention_mil_model.py
  • prostate_cancer/callbacks/__init__.py
  • prostate_cancer/callbacks/curves_callback_base.py
  • prostate_cancer/callbacks/curves_callback_mil.py
  • prostate_cancer/callbacks/curves_callback_sl.py
  • prostate_cancer/callbacks/curves_callback_tl.py
  • prostate_cancer/callbacks/mil_prediction_callback.py
  • prostate_cancer/callbacks/nested_metrics_callback.py
  • prostate_cancer/callbacks/nested_metrics_callback_base.py
  • prostate_cancer/callbacks/nested_metrics_callback_mil.py
  • prostate_cancer/callbacks/slide_histograms_callback_mil.py
  • prostate_cancer/callbacks/tile_histograms_callback_base.py
  • prostate_cancer/callbacks/tile_histograms_callback_mil.py
  • prostate_cancer/callbacks/tile_histograms_callback_tl.py

Comment thread prostate_cancer/callbacks/curves_callback_base.py
Comment thread prostate_cancer/callbacks/nested_metrics_callback_mil.py

@coderabbitai coderabbitai 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.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
prostate_cancer/callbacks/curves_callback_base.py (1)

111-127: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Skip plotting when buffers are empty.

on_test_epoch_end() and on_predict_epoch_end() run on the base classes even when no batch callbacks added data.

  • prostate_cancer/callbacks/curves_callback_base.py#L111-L127: return before torch.cat(...) if self.preds or self.targets is empty.
  • prostate_cancer/callbacks/tile_histograms_callback_base.py#L19-L60: return before np.concatenate(...) if self.all_preds or self.all_labels is empty.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@prostate_cancer/callbacks/curves_callback_base.py` around lines 111 - 127,
Guard _plot_and_clear in prostate_cancer/callbacks/curves_callback_base.py
(lines 111-127) by returning before torch.cat when either self.preds or
self.targets is empty. Apply the same early-return guard in the plotting method
in prostate_cancer/callbacks/tile_histograms_callback_base.py (lines 19-60)
before np.concatenate when either self.all_preds or self.all_labels is empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@configs/model/pgp_hybrid_mil_model.yaml`:
- Around line 3-5: Bind the required lr, tl_threshold, and sl_threshold
parameters for ProstateCancerAttentionMIL using the shared experiment/default
configuration or explicit valid overrides. Apply the same change to
configs/model/pgp_hybrid_mil_model.yaml lines 3-5 and
configs/model/virchow2_hybrid_mil_model.yaml lines 3-5, removing the mandatory
??? placeholders so Hydra can instantiate both models.

In `@prostate_cancer/callbacks/curves_callback_base.py`:
- Line 13: Update CurvesCallbackBase so the ROC and precision-recall artifact
names generated in its plotting methods include the callback level instead of
fixed tile-level names. Also update TileHistogramsCallbackBase to include its
slide/tile level in the histograms artifact name, or move that naming into
subclasses, ensuring artifacts do not collide in the shared MLflow plots path.
Apply the changes in prostate_cancer/callbacks/curves_callback_base.py (anchor,
line 13) and prostate_cancer/callbacks/tile_histograms_callback_base.py
(sibling, lines 12-19).

In `@prostate_cancer/callbacks/nested_metrics_callback_mil.py`:
- Around line 26-31: Update the keys construction in the nested metrics callback
to call nonzero() once on mask_bool, then use the resulting selected row indices
to assemble slide names from metadata. Remove the nested per-tile iteration and
scalar if condition so GPU synchronization occurs only during the single index
materialization.

---

Outside diff comments:
In `@prostate_cancer/callbacks/curves_callback_base.py`:
- Around line 111-127: Guard _plot_and_clear in
prostate_cancer/callbacks/curves_callback_base.py (lines 111-127) by returning
before torch.cat when either self.preds or self.targets is empty. Apply the same
early-return guard in the plotting method in
prostate_cancer/callbacks/tile_histograms_callback_base.py (lines 19-60) before
np.concatenate when either self.all_preds or self.all_labels is empty.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 03830ea8-6940-46ae-ba68-6685b68d0e18

📥 Commits

Reviewing files that changed from the base of the PR and between 3a07ef2 and ae45736.

📒 Files selected for processing (49)
  • configs/callbacks/combined_test_suite_mil.yaml
  • configs/callbacks/curves.yaml
  • configs/callbacks/curves_mil.yaml
  • configs/callbacks/curves_sl.yaml
  • configs/callbacks/nested_metrics_mil.yaml
  • configs/callbacks/slide_histograms_mil.yaml
  • configs/callbacks/slide_threshold_mil.yaml
  • configs/callbacks/tile_histograms.yaml
  • configs/callbacks/tile_histograms_mil.yaml
  • configs/callbacks/tile_threshold_mil.yaml
  • configs/experiment/predict/bag_of_embeddings/pgp/base.yaml
  • configs/experiment/predict/bag_of_embeddings/pgp/mil_predictions/base.yaml
  • configs/experiment/predict/bag_of_embeddings/pgp/mil_predictions/mmci_sl_group_1.yaml
  • configs/experiment/predict/bag_of_embeddings/pgp/mil_predictions/mmci_tl_val.yaml
  • configs/experiment/predict/bag_of_embeddings/pgp/slide_threshold.yaml
  • configs/experiment/predict/bag_of_embeddings/virchow2/base.yaml
  • configs/experiment/predict/bag_of_embeddings/virchow2/mil_predictions/mmci_sl_group_1.yaml
  • configs/experiment/predict/bag_of_embeddings/virchow2/slide_threshold.yaml
  • configs/experiment/test/bag_of_embeddings/base.yaml
  • configs/experiment/test/bag_of_embeddings/pgp/base.yaml
  • configs/experiment/test/bag_of_embeddings/pgp/test_suite/base.yaml
  • configs/experiment/test/bag_of_embeddings/pgp/test_suite/mmci_tl_test.yaml
  • configs/experiment/test/bag_of_embeddings/pgp/tile_threshold.yaml
  • configs/experiment/test/bag_of_embeddings/virchow2/base.yaml
  • configs/experiment/test/bag_of_embeddings/virchow2/test_suite/base.yaml
  • configs/experiment/test/bag_of_embeddings/virchow2/test_suite/mmci_tl_test.yaml
  • configs/experiment/test/bag_of_embeddings/virchow2/tile_threshold.yaml
  • configs/experiment/train/bag_of_embeddings/pgp.yaml
  • configs/experiment/train/bag_of_embeddings/virchow2.yaml
  • configs/model/checkpoints/mmci_tl/pgp_hybrid_mil.yaml
  • configs/model/checkpoints/mmci_tl/virchow2_hybrid_mil.yaml
  • configs/model/pgp_hybrid_mil_model.yaml
  • configs/model/virchow2_hybrid_mil_model.yaml
  • preprocessing/embeddings/tile_embeddings.py
  • preprocessing/masks/quality_control_v2.py
  • prostate_cancer/attention_mil_model.py
  • prostate_cancer/callbacks/__init__.py
  • prostate_cancer/callbacks/curves_callback_base.py
  • prostate_cancer/callbacks/curves_callback_mil.py
  • prostate_cancer/callbacks/curves_callback_sl.py
  • prostate_cancer/callbacks/curves_callback_tl.py
  • prostate_cancer/callbacks/mil_prediction_callback.py
  • prostate_cancer/callbacks/nested_metrics_callback.py
  • prostate_cancer/callbacks/nested_metrics_callback_base.py
  • prostate_cancer/callbacks/nested_metrics_callback_mil.py
  • prostate_cancer/callbacks/slide_histograms_callback_mil.py
  • prostate_cancer/callbacks/tile_histograms_callback_base.py
  • prostate_cancer/callbacks/tile_histograms_callback_mil.py
  • prostate_cancer/callbacks/tile_histograms_callback_tl.py
🛑 Comments failed to post (1)
configs/model/pgp_hybrid_mil_model.yaml (1)

3-5: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

rg -n -C2 '^(lr|tl_threshold|sl_threshold): \?\?\?$|model\.(lr|tl_threshold|sl_threshold)' \
  configs/model/pgp_hybrid_mil_model.yaml \
  configs/model/virchow2_hybrid_mil_model.yaml \
  configs/experiment/train/bag_of_embeddings/pgp.yaml \
  configs/experiment/train/bag_of_embeddings/virchow2.yaml

Repository: RationAI/prostate-cancer

Length of output: 885


🏁 Script executed:

#!/bin/bash
set -eu

echo "== candidate config files =="
for f in configs/config.yaml configs/experiment/train/bag_of_embeddings/pgp.yaml configs/experiment/train/bag_of_embeddings/virchow2.yaml configs/model/*.yaml; do
  [ -f "$f" ] || continue
  echo "--- $f"
  sed -n '1,120p' "$f"
done

echo
echo "== model class constructor/defaults =="
rg -n "class ProstateCancerAttentionMIL|def __init__|lr|tl_threshold|sl_threshold|foundation" prostate_cancer/attention_mil_model.py
sed -n '1,220p' prostate_cancer/attention_mil_model.py

echo
echo "== usages of trainer/model config keys =="
rg -n "model=|--model=|model:" configs --glob '*.yaml'

Repository: RationAI/prostate-cancer

Length of output: 11757


Bind the required hybrid-model parameters.

ProstateCancerAttentionMIL requires lr, tl_threshold, and sl_threshold; these selected configs currently leave them as mandatory ???, so Hydra instantiation fails before training starts. Wire these to the shared experiment/default learning-rate and threshold configuration or provide fixed value overrides.

  • configs/model/pgp_hybrid_mil_model.yaml#L3-L5
  • configs/model/virchow2_hybrid_mil_model.yaml#L3-L5
📍 Affects 2 files
  • configs/model/pgp_hybrid_mil_model.yaml#L3-L5 (this comment)
  • configs/model/virchow2_hybrid_mil_model.yaml#L3-L5
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@configs/model/pgp_hybrid_mil_model.yaml` around lines 3 - 5, Bind the
required lr, tl_threshold, and sl_threshold parameters for
ProstateCancerAttentionMIL using the shared experiment/default configuration or
explicit valid overrides. Apply the same change to
configs/model/pgp_hybrid_mil_model.yaml lines 3-5 and
configs/model/virchow2_hybrid_mil_model.yaml lines 3-5, removing the mandatory
??? placeholders so Hydra can instantiate both models.

Comment thread prostate_cancer/callbacks/slide_histograms_callback_mil.py Outdated

@coderabbitai coderabbitai 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
prostate_cancer/callbacks/curves_callback_base.py (1)

119-135: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Handle empty evaluation buffers before concatenation.

Both shared bases assume that every test or prediction epoch collects usable data. Empty loaders can make torch.cat() or np.concatenate() fail. Add an empty-data path before concatenation, and keep a separate mismatch error when only one buffer is populated.

  • prostate_cancer/callbacks/curves_callback_base.py#L119-L135: guard empty or zero-sample curve buffers before calling _plot_roc() and _plot_precision_recall().
  • prostate_cancer/callbacks/histograms_callback_base.py#L19-L23: guard empty histogram buffers before calling np.concatenate().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@prostate_cancer/callbacks/curves_callback_base.py` around lines 119 - 135,
Update _plot_and_clear in prostate_cancer/callbacks/curves_callback_base.py at
lines 119-135 to detect empty or zero-sample preds and targets before
torch.cat(), return without plotting when both are empty, and raise a separate
mismatch error when only one buffer is populated; retain clearing behavior as
appropriate. Update the histogram buffer handling in
prostate_cancer/callbacks/histograms_callback_base.py at lines 19-23 to guard
empty buffers before np.concatenate().
prostate_cancer/callbacks/curves_callback_sl.py (1)

20-28: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Reject a missing Trainer.datamodule value, not only a missing attribute.

PyTorch Lightning can expose trainer.datamodule with None, so hasattr() can pass and trainer.datamodule.predict.slides can still fail. Read the value with getattr(), reject None, then cast it.

Proposed fix
-        if not hasattr(trainer, "datamodule"):
+        datamodule = getattr(trainer, "datamodule", None)
+        if datamodule is None:
             raise ValueError("Trainer should have datamodule attribute")

-        datamodule = cast("BagOfTilesDataModule", trainer.datamodule)
+        datamodule = cast("BagOfTilesDataModule", datamodule)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@prostate_cancer/callbacks/curves_callback_sl.py` around lines 20 - 28, Update
setup in the callback to retrieve trainer.datamodule via getattr with a None
default, reject a missing or None value with the existing ValueError, and only
then cast it to BagOfTilesDataModule before accessing predict.slides.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@prostate_cancer/callbacks/curves_callback_base.py`:
- Around line 119-135: Update _plot_and_clear in
prostate_cancer/callbacks/curves_callback_base.py at lines 119-135 to detect
empty or zero-sample preds and targets before torch.cat(), return without
plotting when both are empty, and raise a separate mismatch error when only one
buffer is populated; retain clearing behavior as appropriate. Update the
histogram buffer handling in
prostate_cancer/callbacks/histograms_callback_base.py at lines 19-23 to guard
empty buffers before np.concatenate().

In `@prostate_cancer/callbacks/curves_callback_sl.py`:
- Around line 20-28: Update setup in the callback to retrieve trainer.datamodule
via getattr with a None default, reject a missing or None value with the
existing ValueError, and only then cast it to BagOfTilesDataModule before
accessing predict.slides.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5cfc05bb-4c2c-4986-b043-f996d4169e5a

📥 Commits

Reviewing files that changed from the base of the PR and between ae45736 and 7075189.

📒 Files selected for processing (10)
  • prostate_cancer/attention_mil_model.py
  • prostate_cancer/callbacks/curves_callback_base.py
  • prostate_cancer/callbacks/curves_callback_mil.py
  • prostate_cancer/callbacks/curves_callback_sl.py
  • prostate_cancer/callbacks/curves_callback_tl.py
  • prostate_cancer/callbacks/histograms_callback_base.py
  • prostate_cancer/callbacks/mil_prediction_callback.py
  • prostate_cancer/callbacks/slide_histograms_callback_mil.py
  • prostate_cancer/callbacks/tile_histograms_callback_mil.py
  • prostate_cancer/callbacks/tile_histograms_callback_tl.py
🚧 Files skipped from review as they are similar to previous changes (4)
  • prostate_cancer/attention_mil_model.py
  • prostate_cancer/callbacks/slide_histograms_callback_mil.py
  • prostate_cancer/callbacks/mil_prediction_callback.py
  • prostate_cancer/callbacks/tile_histograms_callback_mil.py

@ejdam87
ejdam87 merged commit 83e6131 into master Aug 3, 2026
3 checks passed
@ejdam87
ejdam87 deleted the experiment/hybrid-mil-eval branch August 3, 2026 09:18
@coderabbitai coderabbitai Bot mentioned this pull request Aug 3, 2026
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