ENH: Update fit-to-patient for non-contrast heart models#87
Conversation
Non-contrast heart models from TotalSegmentator are incomplete, but this update makes registration to a statistical model possible.
WalkthroughThe changes migrate workflow execution to ChangesWorkflow API and callers
Segmentation and utility behavior
DirLab-4DCT data flow
Estimated code review effort: 4 (Complex) | ~45 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/physiotwin4d/workflow_convert_image_to_usd.py (1)
83-84: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the docstring to reflect the new default value.
The default value for
frames_per_secondwas changed to1.0(at line 57), but the docstring still states it defaults to24.0. As per coding guidelines, docstrings must be updated and kept factual for changed public methods.📝 Proposed fix
- frames_per_second: Frames per second for animated USD time series. - Defaults to 24.0, matching the underlying VTK-to-USD converter. + frames_per_second: Frames per second for animated USD time series. + Defaults to 1.0.🤖 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 `@src/physiotwin4d/workflow_convert_image_to_usd.py` around lines 83 - 84, Update the frames_per_second parameter documentation in the workflow conversion function’s docstring to state that the default is 1.0 instead of 24.0, matching the value defined in the function signature.Source: Coding guidelines
docs/developer/segmentation.rst (1)
40-44: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDoc references stale
contrastid 135.
segment_chest_total_segmentator_with_contrast.pynow registers"contrast"at id 155 (moved from 135 to avoid colliding with the newbody_extremities=135 label), but this line still documents "contrast at id 135".🤖 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 `@docs/developer/segmentation.rst` around lines 40 - 44, Update the taxonomy documentation for the default “contrast” placeholder to use id 155 instead of 135, while retaining the “soft_tissue” id 133 reference and the surrounding description.src/physiotwin4d/segment_chest_total_segmentator.py (1)
46-47: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore
target_spacingto1.5or update the contract around resampling.self.target_spacing = 0.0disablesSegmentAnatomyBase.preprocess_input(), contradicts the class docstring, and breaks the current initialization test that expects1.5. If the new behavior is intentional, update the docstring and test to match.🤖 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 `@src/physiotwin4d/segment_chest_total_segmentator.py` around lines 46 - 47, Restore self.target_spacing to 1.5 in the TotalSegmentator class so it matches the documented contract and existing initialization test, preserving the expected resampling behavior through SegmentAnatomyBase.preprocess_input().
🧹 Nitpick comments (3)
src/physiotwin4d/contour_tools.py (1)
34-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate docstrings for the changed public methods.
The static-method conversion is compatible with the supplied callers, but the public documentation is incomplete:
transform_contoursomits several parameters,create_reference_imagelacks argument and return documentation, andcreate_mask_from_meshhas no docstring. Please update all five changed methods with complete, factual docstrings.As per coding guidelines, update docstrings for every changed public method.
Also applies to: 74-75, 138-139, 167-168, 363-364
🤖 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 `@src/physiotwin4d/contour_tools.py` around lines 34 - 35, Update the docstrings for all five changed public methods, including extract_contours, transform_contours, create_reference_image, create_mask_from_mesh, and the method at the additional referenced location. Document every parameter and the return value with accurate descriptions, ensuring transform_contours includes its omitted parameters and create_reference_image and create_mask_from_mesh have complete argument and return documentation.Source: Coding guidelines
src/physiotwin4d/segment_anatomy_base.py (1)
456-463: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGroup labelmaps aren't cast to uint8 like the main
"labelmap"key.
labelmap_imageis explicitly re-cast touint8before being returned, but thelabelmapsdict (heart/lung/etc.) is built from the pre-cast image and returned as-is. All current segmenters happen to already produce uint8 labelmaps, so this is not currently observable, but the returned dict has no guaranteed-consistent pixel type across its values, which downstream consumers (writers, comparisons) may implicitly assume.♻️ Suggested fix
labelmaps = self.create_anatomy_group_labelmaps(labelmap_image) labelmap_image = itk.GetImageFromArray( itk.GetArrayFromImage(labelmap_image).astype(np.uint8) ) labelmap_image.CopyInformation(input_image) - return {"labelmap": labelmap_image, **labelmaps} + labelmaps = { + group: itk.GetImageFromArray( + itk.GetArrayFromImage(group_labelmap).astype(np.uint8) + ) + for group, group_labelmap in labelmaps.items() + } + for group_labelmap, source in zip(labelmaps.values(), labelmaps.values()): + group_labelmap.CopyInformation(input_image) + + return {"labelmap": labelmap_image, **labelmaps}🤖 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 `@src/physiotwin4d/segment_anatomy_base.py` around lines 456 - 463, Update the labelmap construction flow around create_anatomy_group_labelmaps so every group labelmap in labelmaps is converted to uint8 before returning. Apply the same pixel-type normalization used for the main "labelmap" while preserving each group's existing image metadata and the return structure.tutorials/tutorial_04_fit_statistical_model_to_patient.py (1)
101-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPersisted ITK images/transforms omit
compression=Truein two tutorials. Bothitk.imwriteanditk.transformwritedefault theircompressionparameter toFalse; the coding guidelines require compressed storage for both ITK images and.hdftransforms.
tutorials/tutorial_04_fit_statistical_model_to_patient.py#L101-L110: addcompression=Trueto the threeitk.imwrite(...)calls (patient image, patient labelmap, heart labelmap).tutorials/tutorial_06_reconstruct_highres_4d_ct.py#L107-L111: addcompression=Trueto the twoitk.transformwrite(...)calls (forward and inverse transform).🤖 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 `@tutorials/tutorial_04_fit_statistical_model_to_patient.py` around lines 101 - 110, Enable compression on all persisted ITK outputs: update the three itk.imwrite calls in tutorials/tutorial_04_fit_statistical_model_to_patient.py for patient_image, patient_labelmap, and heart_labelmap, and the two itk.transformwrite calls in tutorials/tutorial_06_reconstruct_highres_4d_ct.py for the forward and inverse transforms, by passing compression=True.Source: Coding guidelines
🤖 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 `@experiments/Heart-Create_Statistical_Model/README.md`:
- Line 164: Update the workflow result assignment around
WorkflowFitStatisticalModelToPatient.process() so it selects
registered_template_model_surface from the returned dictionary instead of
assigning the entire dictionary to registered_mesh.
In `@src/physiotwin4d/segment_chest_total_segmentator.py`:
- Line 286: Remove the three unconditional debug itk.imwrite calls for
preprocessed_image, labelmap_arr_heart, and labelmap_arr_body from the
segmentation flow. Do not replace them with other production writes; preserve
the segmentation behavior while eliminating the working-directory artifacts and
associated debug image creation if it is used only by those writes.
- Around line 369-393: Binarize the heart interior mask before constructing
interior_image in the high-resolution heart expansion block: make interior_arr
contain 1 for labels 141–144 and 0 elsewhere, while preserving the existing
dilation and erosion calls using foreground value 1. Keep the subsequent
exterior mask application and labelmap replacement behavior unchanged.
In `@src/physiotwin4d/workflow_convert_image_to_usd.py`:
- Line 228: Wrap the assignment to self.reference_segmentation_results in the
workflow conversion code so each resulting line is no longer than 88 characters,
preserving the existing segmenter.segment(self.reference_image) behavior.
In `@tests/conftest.py`:
- Around line 517-529: Remove the unconditional "contrast" lookup from the
fixture result assembled with segmenter_total_segmentator, since
create_anatomy_group_labelmaps only returns registered groups. Keep the existing
groups unchanged; if contrast output is required, use the
segmenter_total_segmentator_with_contrast fixture instead.
In `@tutorials/tutorial_04_fit_statistical_model_to_patient.py`:
- Around line 128-143: Guard PCA registration in the workflow setup using the
loaded pca_model: call set_use_pca_registration only when pca_model is not None,
while preserving the existing enabled configuration when it is available. Update
the result-saving flow around workflow.pca_coefficients so it does not call
tolist() when PCA was skipped, and save registered coefficients only when they
exist.
---
Outside diff comments:
In `@docs/developer/segmentation.rst`:
- Around line 40-44: Update the taxonomy documentation for the default
“contrast” placeholder to use id 155 instead of 135, while retaining the
“soft_tissue” id 133 reference and the surrounding description.
In `@src/physiotwin4d/segment_chest_total_segmentator.py`:
- Around line 46-47: Restore self.target_spacing to 1.5 in the TotalSegmentator
class so it matches the documented contract and existing initialization test,
preserving the expected resampling behavior through
SegmentAnatomyBase.preprocess_input().
In `@src/physiotwin4d/workflow_convert_image_to_usd.py`:
- Around line 83-84: Update the frames_per_second parameter documentation in the
workflow conversion function’s docstring to state that the default is 1.0
instead of 24.0, matching the value defined in the function signature.
---
Nitpick comments:
In `@src/physiotwin4d/contour_tools.py`:
- Around line 34-35: Update the docstrings for all five changed public methods,
including extract_contours, transform_contours, create_reference_image,
create_mask_from_mesh, and the method at the additional referenced location.
Document every parameter and the return value with accurate descriptions,
ensuring transform_contours includes its omitted parameters and
create_reference_image and create_mask_from_mesh have complete argument and
return documentation.
In `@src/physiotwin4d/segment_anatomy_base.py`:
- Around line 456-463: Update the labelmap construction flow around
create_anatomy_group_labelmaps so every group labelmap in labelmaps is converted
to uint8 before returning. Apply the same pixel-type normalization used for the
main "labelmap" while preserving each group's existing image metadata and the
return structure.
In `@tutorials/tutorial_04_fit_statistical_model_to_patient.py`:
- Around line 101-110: Enable compression on all persisted ITK outputs: update
the three itk.imwrite calls in
tutorials/tutorial_04_fit_statistical_model_to_patient.py for patient_image,
patient_labelmap, and heart_labelmap, and the two itk.transformwrite calls in
tutorials/tutorial_06_reconstruct_highres_4d_ct.py for the forward and inverse
transforms, by passing compression=True.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: abdc6a98-e2d4-4726-9063-94a05139b70e
📒 Files selected for processing (23)
docs/api/model_registration/index.rstdocs/api/workflows.rstdocs/developer/registration_models.rstdocs/developer/segmentation.rstdocs/tutorials.rstexperiments/Heart-Create_Statistical_Model/README.mdexperiments/Heart-GatedCT_To_USD/2-generate_segmentation.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient-CHOPValve.pysrc/physiotwin4d/cli/fit_statistical_model_to_patient.pysrc/physiotwin4d/cli/reconstruct_highres_4d_ct.pysrc/physiotwin4d/contour_tools.pysrc/physiotwin4d/segment_anatomy_base.pysrc/physiotwin4d/segment_chest_total_segmentator.pysrc/physiotwin4d/segment_chest_total_segmentator_with_contrast.pysrc/physiotwin4d/segment_heart_simpleware.pysrc/physiotwin4d/workflow_convert_image_to_usd.pysrc/physiotwin4d/workflow_fit_statistical_model_to_patient.pysrc/physiotwin4d/workflow_reconstruct_highres_4d_ct.pytests/conftest.pytests/test_segment_chest_total_segmentator.pytutorials/tutorial_04_fit_statistical_model_to_patient.pytutorials/tutorial_06_reconstruct_highres_4d_ct.pytutorials/tutorial_08_cardiac_fit_model.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tutorials/tutorial_01b_lung_gated_ct_to_usd.py (1)
141-150: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueChange
usd_project_nameto reflect lung anatomy.Since this tutorial processes lung data from DirLab, consider renaming the USD project name from
"cardiac_model"to"lung_model"(or similar) to avoid confusion in the output artifacts.♻️ Proposed fix
workflow = WorkflowConvertImageToUSD( time_series_images=time_series_images, reference_image=reference_image, output_directory=str(output_dir), - usd_project_name="cardiac_model", + usd_project_name="lung_model", registration_method=registration_method, log_level=log_level, save_assets=True, )🤖 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 `@tutorials/tutorial_01b_lung_gated_ct_to_usd.py` around lines 141 - 150, Update the usd_project_name argument in the WorkflowConvertImageToUSD initialization to use a lung-specific name such as "lung_model" instead of "cardiac_model", keeping the remaining workflow configuration unchanged.
🤖 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 `@tutorials/tutorial_01b_lung_gated_ct_to_usd.py`:
- Around line 1-68: Update the module-level docstring to describe the
DirLab-4DCT lung workflow rather than cardiac processing. Replace heart-specific
title, anatomy, dataset, input path, terminology, class descriptions, and output
filenames with accurate lung tutorial details, while preserving the documented
workflow purpose, requirements, and limitations where applicable.
---
Nitpick comments:
In `@tutorials/tutorial_01b_lung_gated_ct_to_usd.py`:
- Around line 141-150: Update the usd_project_name argument in the
WorkflowConvertImageToUSD initialization to use a lung-specific name such as
"lung_model" instead of "cardiac_model", keeping the remaining workflow
configuration unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4025c926-9f8f-42b0-8e87-1afd927394ca
📒 Files selected for processing (113)
data/DirLab-4DCT/README.mddata/DirLab-4DCT/downloaded_data/Case10Pack_T00.mhddata/DirLab-4DCT/downloaded_data/Case10Pack_T10.mhddata/DirLab-4DCT/downloaded_data/Case10Pack_T20.mhddata/DirLab-4DCT/downloaded_data/Case10Pack_T30.mhddata/DirLab-4DCT/downloaded_data/Case10Pack_T40.mhddata/DirLab-4DCT/downloaded_data/Case10Pack_T50.mhddata/DirLab-4DCT/downloaded_data/Case10Pack_T60.mhddata/DirLab-4DCT/downloaded_data/Case10Pack_T70.mhddata/DirLab-4DCT/downloaded_data/Case10Pack_T80.mhddata/DirLab-4DCT/downloaded_data/Case10Pack_T90.mhddata/DirLab-4DCT/downloaded_data/Case1Pack_T00.mhddata/DirLab-4DCT/downloaded_data/Case1Pack_T10.mhddata/DirLab-4DCT/downloaded_data/Case1Pack_T20.mhddata/DirLab-4DCT/downloaded_data/Case1Pack_T30.mhddata/DirLab-4DCT/downloaded_data/Case1Pack_T40.mhddata/DirLab-4DCT/downloaded_data/Case1Pack_T50.mhddata/DirLab-4DCT/downloaded_data/Case1Pack_T60.mhddata/DirLab-4DCT/downloaded_data/Case1Pack_T70.mhddata/DirLab-4DCT/downloaded_data/Case1Pack_T80.mhddata/DirLab-4DCT/downloaded_data/Case1Pack_T90.mhddata/DirLab-4DCT/downloaded_data/Case2Pack_T00.mhddata/DirLab-4DCT/downloaded_data/Case2Pack_T10.mhddata/DirLab-4DCT/downloaded_data/Case2Pack_T20.mhddata/DirLab-4DCT/downloaded_data/Case2Pack_T30.mhddata/DirLab-4DCT/downloaded_data/Case2Pack_T40.mhddata/DirLab-4DCT/downloaded_data/Case2Pack_T50.mhddata/DirLab-4DCT/downloaded_data/Case2Pack_T60.mhddata/DirLab-4DCT/downloaded_data/Case2Pack_T70.mhddata/DirLab-4DCT/downloaded_data/Case2Pack_T80.mhddata/DirLab-4DCT/downloaded_data/Case2Pack_T90.mhddata/DirLab-4DCT/downloaded_data/Case3Pack_T00.mhddata/DirLab-4DCT/downloaded_data/Case3Pack_T10.mhddata/DirLab-4DCT/downloaded_data/Case3Pack_T20.mhddata/DirLab-4DCT/downloaded_data/Case3Pack_T30.mhddata/DirLab-4DCT/downloaded_data/Case3Pack_T40.mhddata/DirLab-4DCT/downloaded_data/Case3Pack_T50.mhddata/DirLab-4DCT/downloaded_data/Case3Pack_T60.mhddata/DirLab-4DCT/downloaded_data/Case3Pack_T70.mhddata/DirLab-4DCT/downloaded_data/Case3Pack_T80.mhddata/DirLab-4DCT/downloaded_data/Case3Pack_T90.mhddata/DirLab-4DCT/downloaded_data/Case4Pack_T00.mhddata/DirLab-4DCT/downloaded_data/Case4Pack_T10.mhddata/DirLab-4DCT/downloaded_data/Case4Pack_T20.mhddata/DirLab-4DCT/downloaded_data/Case4Pack_T30.mhddata/DirLab-4DCT/downloaded_data/Case4Pack_T40.mhddata/DirLab-4DCT/downloaded_data/Case4Pack_T50.mhddata/DirLab-4DCT/downloaded_data/Case4Pack_T60.mhddata/DirLab-4DCT/downloaded_data/Case4Pack_T70.mhddata/DirLab-4DCT/downloaded_data/Case4Pack_T80.mhddata/DirLab-4DCT/downloaded_data/Case4Pack_T90.mhddata/DirLab-4DCT/downloaded_data/Case5Pack_T00.mhddata/DirLab-4DCT/downloaded_data/Case5Pack_T10.mhddata/DirLab-4DCT/downloaded_data/Case5Pack_T20.mhddata/DirLab-4DCT/downloaded_data/Case5Pack_T30.mhddata/DirLab-4DCT/downloaded_data/Case5Pack_T40.mhddata/DirLab-4DCT/downloaded_data/Case5Pack_T50.mhddata/DirLab-4DCT/downloaded_data/Case5Pack_T60.mhddata/DirLab-4DCT/downloaded_data/Case5Pack_T70.mhddata/DirLab-4DCT/downloaded_data/Case5Pack_T80.mhddata/DirLab-4DCT/downloaded_data/Case5Pack_T90.mhddata/DirLab-4DCT/downloaded_data/Case6Pack_T00.mhddata/DirLab-4DCT/downloaded_data/Case6Pack_T10.mhddata/DirLab-4DCT/downloaded_data/Case6Pack_T20.mhddata/DirLab-4DCT/downloaded_data/Case6Pack_T30.mhddata/DirLab-4DCT/downloaded_data/Case6Pack_T40.mhddata/DirLab-4DCT/downloaded_data/Case6Pack_T50.mhddata/DirLab-4DCT/downloaded_data/Case6Pack_T60.mhddata/DirLab-4DCT/downloaded_data/Case6Pack_T70.mhddata/DirLab-4DCT/downloaded_data/Case6Pack_T80.mhddata/DirLab-4DCT/downloaded_data/Case6Pack_T90.mhddata/DirLab-4DCT/downloaded_data/Case7Pack_T00.mhddata/DirLab-4DCT/downloaded_data/Case7Pack_T10.mhddata/DirLab-4DCT/downloaded_data/Case7Pack_T20.mhddata/DirLab-4DCT/downloaded_data/Case7Pack_T30.mhddata/DirLab-4DCT/downloaded_data/Case7Pack_T40.mhddata/DirLab-4DCT/downloaded_data/Case7Pack_T50.mhddata/DirLab-4DCT/downloaded_data/Case7Pack_T60.mhddata/DirLab-4DCT/downloaded_data/Case7Pack_T70.mhddata/DirLab-4DCT/downloaded_data/Case7Pack_T80.mhddata/DirLab-4DCT/downloaded_data/Case7Pack_T90.mhddata/DirLab-4DCT/downloaded_data/Case8Deploy_T00.mhddata/DirLab-4DCT/downloaded_data/Case8Deploy_T10.mhddata/DirLab-4DCT/downloaded_data/Case8Deploy_T20.mhddata/DirLab-4DCT/downloaded_data/Case8Deploy_T30.mhddata/DirLab-4DCT/downloaded_data/Case8Deploy_T40.mhddata/DirLab-4DCT/downloaded_data/Case8Deploy_T50.mhddata/DirLab-4DCT/downloaded_data/Case8Deploy_T60.mhddata/DirLab-4DCT/downloaded_data/Case8Deploy_T70.mhddata/DirLab-4DCT/downloaded_data/Case8Deploy_T80.mhddata/DirLab-4DCT/downloaded_data/Case8Deploy_T90.mhddata/DirLab-4DCT/downloaded_data/Case9Pack_T00.mhddata/DirLab-4DCT/downloaded_data/Case9Pack_T10.mhddata/DirLab-4DCT/downloaded_data/Case9Pack_T20.mhddata/DirLab-4DCT/downloaded_data/Case9Pack_T30.mhddata/DirLab-4DCT/downloaded_data/Case9Pack_T40.mhddata/DirLab-4DCT/downloaded_data/Case9Pack_T50.mhddata/DirLab-4DCT/downloaded_data/Case9Pack_T60.mhddata/DirLab-4DCT/downloaded_data/Case9Pack_T70.mhddata/DirLab-4DCT/downloaded_data/Case9Pack_T80.mhddata/DirLab-4DCT/downloaded_data/Case9Pack_T90.mhddata/DirLab-4DCT/downloaded_data/Convert4DCTToMHD.pydata/DirLab-4DCT/fix_downloaded_data.pyexperiments/Heart-Create_Statistical_Model/README.mdexperiments/Lung-GatedCT_To_USD/0-register_dirlab_4dct.pyexperiments/Lung-GatedCT_To_USD/Experiment_SegReg.pyexperiments/Lung-GatedCT_To_USD/data_dirlab_4d_ct.pysrc/physiotwin4d/data_download_tools.pysrc/physiotwin4d/segment_chest_total_segmentator.pytests/conftest.pytutorials/tutorial_01b_lung_gated_ct_to_usd.pytutorials/tutorial_04_fit_statistical_model_to_patient.pytutorials/tutorial_06_reconstruct_highres_4d_ct.py
💤 Files with no reviewable changes (1)
- tests/conftest.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tutorials/tutorial_04_fit_statistical_model_to_patient.py
- src/physiotwin4d/segment_chest_total_segmentator.py
Non-contrast heart models from TotalSegmentator are incomplete, but this update makes registration to a statistical model possible.
Summary by CodeRabbit
process()entry point (and refreshed related examples/terminology).