ENH: AnatomyTaxonomy, USD pipeline hardening, tutorials + docs#49
Merged
Conversation
Replaces the per-organ flat dicts on SegmentAnatomyBase and USDAnatomyTools
with a shared AnatomyTaxonomy data type so new segmenters can register
custom groups via taxonomy.add_organ without subclassing either class.
Adds anatomy-type grouping to ConvertVTKToUSD's labeled output, ships
tutorials 07-09 (DirLab PCA + PhysicsNeMo), and fixes several USD-
pipeline issues that prevented clean rendering in Omniverse Kit.
Anatomy / segmentation
- AnatomyTaxonomy + AnatomyGroup (new). SegmentAnatomyBase owns one;
subclasses populate via taxonomy.add_organ and call
_finalize_other_group. Eight legacy *_mask_ids attributes and
set_other_and_all_mask_ids removed; label_to_type delegates to the
taxonomy. segment() now returns only the groups the segmenter
registered (caller-checks contract; tests updated).
- SegmentHeartSimpleware: graceful fallback when ASCardio omits
landmarks.csv; only the groups it actually populates are returned.
USD generation
- ConvertVTKToUSD: new segmenter= parameter groups labeled prims under
/World/{basename}/{type}/{label} and materials under
/World/Looks/{type}/{label}_material. Preserves cell-data primvars;
new compute_von_mises_stress helper for 9-component stress tensors;
framing camera with tight near-clip; stale USD-layer eviction;
triangulation face-map propagated to uniform primvars so per-cell
arrays survive quad-to-triangle splits.
- USDAnatomyTools: DEFAULT_RENDER_PARAMS promoted to a module-level
constant so CLIs/tests can enumerate types without a stage; per-
instance deep copy keeps mutations isolated; enhance_meshes now a
single taxonomy loop with organ-level overrides as data instead of
imperative carve-outs. MaterialBindingAPI.Apply() so Omniverse RTX
consumes the binding (was rendering white).
- CLI convert_vtk_to_usd: ANATOMY_TYPES sourced from
DEFAULT_RENDER_PARAMS (auto-syncs with future additions); "kidney"
alias preserved.
Tutorials / experiments
- Tutorials 07-09 added (DirLab PCA lung-lobe model, time-series
propagation, PhysicsNeMo mesh-stage training).
- Tutorials 02-09 gain if __name__ == "__main__" guards (Windows
nnUNet spawn) matching tutorial_01; experiments gain the same
guards plus Path(__file__)-anchored paths; valve scripts wire
compute_von_mises_stress.
- notebook_utils.py removed (last consumers gone); experiment scripts
and READMEs scrubbed of .ipynb references; test runner already on
*.py glob.
Tests, docs, packaging
- New test_anatomy_taxonomy (12 tests); mask_ids / Simpleware
assertions updated to the caller-checks contract; tutorial test
harness rewritten.
- Segmentation .rst pages, developer/segmentation.rst, and developer/
usd_generation.rst updated to document AnatomyTaxonomy, the
type-grouped layout, von Mises helper, framing camera, and the
three ways to register a new group's look.
- New CLI .rst pages, simpleware/greedy API pages, vtk_to_usd_lib
page, data_download / test_tools utility pages; API_MAP.md
regenerated.
- mypy: 7 errors fixed (Optional render-params indexing, redundant
np.asarray casts, pyvista Plotter binding via Any cast,
matplotlib colormap return typing). Full src/ tree clean.
Other
- data_download_tools.py extracted (was notebook_utils.py glue).
- pyproject.toml mypy overrides tightened.
- Stale baseline PNGs removed (regenerated by test_tools at runtime).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces anatomy taxonomy data modeling for segmentation, adds optional dataset download utilities, refactors segmentation classes to use taxonomy instead of hardcoded mask IDs, enhances USD rendering with anatomy-aware materials and von Mises stress derivation, modernizes tutorials to percent-cell Python scripts, and upgrades minimum Python to 3.11 with corresponding CI/documentation updates. ChangesAnatomy Taxonomy System & Segmentation Refactoring
Data Download & Test Infrastructure Modernization
USD Rendering & Visualization Enhancements
Tutorial Script Modernization
Python Version Upgrade & Build Configuration
Documentation Reorganization & Guides
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
This PR modernizes PhysioMotion4D’s segmentation and USD export pipeline by introducing a shared AnatomyTaxonomy data model, hardening VTK→USD conversion for Omniverse rendering correctness, and expanding/refreshing tutorials, experiments, docs, and tests to match the new contracts.
Changes:
- Introduces
AnatomyTaxonomy/AnatomyGroupto replace per-segmenter flat dicts and standardize label grouping across segmentation and USD rendering. - Hardens USD generation (type-grouped labeled prim layout, material binding via
MaterialBindingAPI.Apply, primvar preservation through triangulation, stale-layer eviction, framing camera, derived primvars like von Mises stress). - Adds/updates tutorials (07–09) and refreshes documentation + tests, including new dataset download verification helpers and OpenUSD screenshot rendering without USD imaging plugins.
Reviewed changes
Copilot reviewed 117 out of 123 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| tutorials/tutorial_08_dirlab_pca_time_series.py | New tutorial to register DirLab phases and propagate PCA-fitted meshes through time series. |
| tutorials/tutorial_07_dirlab_pca_model.py | New tutorial to build and fit a surface PCA model of DirLab lung lobes. |
| tutorials/tutorial_06_reconstruct_highres_4d_ct.py | Refactors tutorial 06 into percent-cell script style and updates test-mode parameters and outputs. |
| tutorials/tutorial_05_vtk_to_usd.py | Refactors tutorial 05 to use WorkflowConvertVTKToUSD + OpenUSD screenshot path. |
| tutorials/README.md | Updates tutorial index and new guidance for percent-cell scripts. |
| tests/test_vtk_to_usd_library.py | Adds OpenUSD screenshot test using VTK-based loader path. |
| tests/test_segment_heart_simpleware.py | Updates assertions to use taxonomy-based group labels and new segment() key contract. |
| tests/test_segment_chest_total_segmentator.py | Updates initialization checks to use taxonomy rather than legacy mask id dicts. |
| tests/test_download_heart_data.py | Adds synthetic tests for DataDownloadTools verification helpers; updates Slicer-Heart directory expectations. |
| tests/test_convert_vtk_to_usd.py | Updates expected labeled prim layout and adds segmenter-type grouping coverage. |
| tests/test_anatomy_taxonomy.py | Adds unit tests for AnatomyTaxonomy/AnatomyGroup and base-class seeding behavior. |
| tests/PARALLEL_EXECUTION_GUIDE.md | Updates experiment execution docs from notebooks to scripts. |
| tests/EXPERIMENT_TESTS_SUMMARY.md | Updates experiment runner documentation for .py script execution. |
| tests/EXPERIMENT_TESTS_GUIDE.md | Updates experiment execution docs and recommends TestTools.running_as_test(). |
| tests/EXPERIMENT_FLAG_USAGE.md | Updates environment-flag documentation for new helper (still contains some “notebook” wording). |
| tests/conftest.py | Switches Slicer-Heart download to DataDownloadTools and reorganizes test data caching dirs. |
| src/physiomotion4d/workflow_convert_heart_gated_ct_to_usd.py | Adds save toggles for intermediates, consolidates write helpers, and forwards taxonomy + segmenter into ConvertVTKToUSD. |
| src/physiomotion4d/workflow_convert_ct_to_vtk.py | Moves label lookup to taxonomy via labels_in_group(). |
| src/physiomotion4d/vtk_to_usd/usd_utils.py | Adds triangulation face map + framing camera helper; updates docstrings/typing. |
| src/physiomotion4d/vtk_to_usd/usd_mesh_converter.py | Expands uniform primvars through triangulation and preserves cell arrays for time samples. |
| src/physiomotion4d/vtk_to_usd/primvar_derivations.py | New derived-primvar registry; includes von Mises stress derivation for 9-component stress tensors. |
| src/physiomotion4d/vtk_to_usd/material_manager.py | Uses MaterialBindingAPI.Apply so Omniverse RTX honors material bindings. |
| src/physiomotion4d/vtk_to_usd/converter.py | Adds stale layer eviction and framing camera for file facade conversion. |
| src/physiomotion4d/vtk_to_usd/init.py | Reframes vtk_to_usd as a stable low-level API and exports new helpers. |
| src/physiomotion4d/usd_tools.py | Adds VTK-based OpenUSD loader + framing camera + stale layer eviction in merge paths. |
| src/physiomotion4d/transform_tools.py | Adds stale-layer eviction + framing camera when exporting transform visualizations. |
| src/physiomotion4d/test_tools.py | Adds running_as_test() and OpenUSD screenshot rendering via VTK loader; makes baselines/results dirs optional. |
| src/physiomotion4d/segment_heart_simpleware.py | Migrates label definitions to taxonomy and adds optional landmarks.csv handling. |
| src/physiomotion4d/register_images_ants.py | Disables verbose output in ANTs backend calls. |
| src/physiomotion4d/notebook_utils.py | Removes obsolete notebook_utils helper module. |
| src/physiomotion4d/data_download_tools.py | Adds dataset download/verification helpers (Slicer-Heart auto-download + verifiers). |
| src/physiomotion4d/cli/convert_vtk_to_usd.py | Sources anatomy types from DEFAULT_RENDER_PARAMS for automatic sync. |
| src/physiomotion4d/anatomy_taxonomy.py | New pure-stdlib AnatomyTaxonomy/AnatomyGroup implementation. |
| src/physiomotion4d/init.py | Exports AnatomyTaxonomy/AnatomyGroup and DataDownloadTools from top-level package. |
| README.md | Updates tutorial count, examples, segmentation key contract notes, and DataDownloadTools usage. |
| pyproject.toml | Bumps Python requirement to 3.11, adds deps (PhysicsNeMo, ipykernel), and updates mypy python_version. |
| experiments/Reconstruct4DCT/reconstruct_4d_ct_class.py | Switches test-mode flag helper to TestTools.running_as_test(). |
| experiments/README.md | Updates experiment docs to scripts + TestTools.running_as_test(). |
| experiments/Lung-GatedCT_To_USD/Experiment_SegReg.py | Adds main guard to avoid Windows spawn cascade with nnUNet/TotalSegmentator. |
| experiments/Lung-GatedCT_To_USD/2-paint_dirlab_models.py | Adds main guard and keeps label lookups taxonomy-aligned. |
| experiments/Lung-GatedCT_To_USD/1-make_dirlab_models.py | Adds main guard and forwards segmenter for type-grouped USD layout. |
| experiments/Heart-VTKSeries_To_USD/1-heart_vtkseries_to_usd.py | Adds main guard to prevent Windows spawn cascade; keeps script-format execution. |
| experiments/Heart-VTKSeries_To_USD/0-download_and_convert_4d_to_3d.py | Uses DataDownloadTools instead of urllib download. |
| experiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient-CHOPValve.py | Switches test-mode helper to TestTools.running_as_test(). |
| experiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_registration_pca.py | Switches test-mode helper to TestTools.running_as_test(). |
| experiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_icp_itk.py | Switches test-mode helper to TestTools.running_as_test(). |
| experiments/Heart-Simpleware_Segmentation/simpleware_heart_segmentation.py | Switches to taxonomy label access and TestTools.running_as_test(). |
| experiments/Heart-Simpleware_Segmentation/README.md | Updates references from notebook to script and aligns instructions. |
| experiments/Heart-GatedCT_To_USD/test_compare_registration_speed.py | Switches test-mode helper to TestTools.running_as_test(). |
| experiments/Heart-GatedCT_To_USD/3-transform_dynamic_and_static_contours.py | Adds main guard and forwards segmenter for type-grouped USD layout. |
| experiments/Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.py | Uses DataDownloadTools and Path-based IO. |
| experiments/Heart-Create_Statistical_Model/README.md | Updates pipeline docs from notebooks to scripts. |
| experiments/Heart-Create_Statistical_Model/5-compute_pca_model.py | Switches test-mode helper to TestTools.running_as_test(). |
| experiments/Heart-Create_Statistical_Model/4-surfaces_aligned_correspond_to_pca_inputs.py | Switches test-mode helper to TestTools.running_as_test(). |
| experiments/Heart-Create_Statistical_Model/3-registration_based_correspondence.py | Switches test-mode helper to TestTools.running_as_test(). |
| experiments/Heart-Create_Statistical_Model/2-input_surfaces_to_surfaces_aligned.py | Switches test-mode helper to TestTools.running_as_test(). |
| experiments/Heart-Create_Statistical_Model/1-input_meshes_to_input_surfaces.py | Switches test-mode helper to TestTools.running_as_test(). |
| experiments/Convert_VTK_To_USD/convert_chop_tpv25_valve_to_usd.py | Anchors paths to script, wires von Mises derivation, updates colormap selection. |
| experiments/Convert_VTK_To_USD/convert_chop_alterra_valve_to_usd.py | Anchors paths to script, wires von Mises derivation, updates test-mode naming. |
| experiments/Colormap-VTK_To_USD/colormap_vtk_to_usd.py | Adds progress prints for colormap conversion. |
| docs/tutorials.rst | Expands tutorial set to 9 and updates execution guidance to script-based usage. |
| docs/quickstart.rst | Updates quickstart to percent-cell tutorial execution and DataDownloadTools usage. |
| docs/examples.rst | Updates dataset download snippet to DataDownloadTools. |
| docs/developer/usd_generation.rst | Documents new labeled layout, von Mises derivation, framing camera, and renderer taxonomy integration. |
| docs/developer/segmentation.rst | Documents taxonomy-driven key contract and how to add new segmenters/groups. |
| docs/contributing.rst | Adds uv install guidance but still mentions Python 3.10+ in checklist. |
| docs/api/workflows.rst | Adds explicit module directives for workflow docs. |
| docs/api/utilities/test_tools.rst | New API page for TestTools. |
| docs/api/utilities/index.rst | Adds test_tools and data_download to utilities index. |
| docs/api/utilities/data_download.rst | New API page for DataDownloadTools. |
| docs/api/utilities/contour_tools.rst | Updates navigation links to include test_tools. |
| docs/api/usd/vtk_to_usd_lib.rst | New docs page for low-level vtk_to_usd subpackage. |
| docs/api/usd/vtk_conversion.rst | Adds module directive for ConvertVTKToUSD API docs. |
| docs/api/usd/index.rst | Adds vtk_to_usd_lib to USD docs index. |
| docs/api/segmentation/totalsegmentator.rst | Adds module directive and clarifies returned keys/contract. |
| docs/api/segmentation/simpleware.rst | New API page for Simpleware segmenter and its returned keys. |
| docs/api/segmentation/index.rst | Adds Simpleware docs and updates guidance to membership checks. |
| docs/api/segmentation/base.rst | Documents AnatomyTaxonomy contract + variability of returned mask keys. |
| docs/api/registration/time_series.rst | Adds module directive for time-series registration docs. |
| docs/api/registration/index.rst | Adds greedy module into registration docs toctree. |
| docs/api/registration/icon.rst | Adds module directive for ICON docs. |
| docs/api/registration/greedy.rst | New API page for Greedy registration backend. |
| docs/api/registration/base.rst | Adds module directive for registration base docs. |
| docs/api/registration/ants.rst | Adds module directive for ANTs docs. |
| docs/api/model_registration/pca.rst | Adds module directive for PCA registration docs. |
| docs/api/model_registration/icp.rst | Adds module directive for ICP docs. |
| docs/api/model_registration/icp_itk.rst | Adds module directive for ITK ICP docs. |
| docs/api/model_registration/distance_maps.rst | Adds module directive for distance-map registration docs. |
| docs/api/index.rst | Adds CLI API section to the Sphinx API index. |
| docs/api/cli/visualize_pca_modes.rst | New API page for visualize_pca_modes CLI module. |
| docs/api/cli/reconstruct_highres_4d_ct.rst | New API page for reconstruct_highres_4d_ct CLI module. |
| docs/api/cli/index.rst | New API index page for CLI entry-point modules. |
| docs/api/cli/fit_statistical_model_to_patient.rst | New API page for fit_statistical_model_to_patient CLI module. |
| docs/api/cli/create_statistical_model.rst | New API page for create_statistical_model CLI module. |
| docs/api/cli/convert_vtk_to_usd.rst | New API page for convert_vtk_to_usd CLI module. |
| docs/api/cli/convert_heart_gated_ct_to_usd.rst | New API page for convert_heart_gated_ct_to_usd CLI module. |
| docs/api/cli/convert_ct_to_vtk.rst | New API page for convert_ct_to_vtk CLI module. |
| docs/api/base.rst | Adds module directive for PhysioMotion4DBase docs. |
| data/test/.gitignore | Changes ignored test-data layout to directory-based (slicer_heart / slicer_heart_small). |
| data/Slicer-Heart-CT/download_and_convert.py | Switches download step to DataDownloadTools. |
| data/README.md | Updates dataset instructions and documents DataDownloadTools verifiers. |
| .agents/skills/skills/caveman/SKILL.md | Adds “caveman” skill markdown documentation. |
| .agents/.gitignore | Ignores additional local agent config artifacts. |
Comments suppressed due to low confidence (1)
src/physiomotion4d/data_download_tools.py:107
- The method name
VerityKCLHeartModelDataappears to be a typo and duplicatesVerifyKCLHeartModelData. If backward compatibility is not required, consider removing it to avoid expanding the public API surface; if it is required, consider marking it deprecated and documenting why it exists.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+65
to
+69
| if time_code is None: | ||
| usd_time = Usd.TimeCode.Default() | ||
| else: | ||
| usd_time = Usd.TimeCode(time_code) | ||
| xform_cache = UsdGeom.XformCache(usd_time) |
Comment on lines
+445
to
+449
| """Render OpenUSD mesh geometry off-screen and save a PNG. | ||
|
|
||
| The scene is loaded through ``USDTools.load_openusd_as_vtk`` into a | ||
| PyVista mesh, rendered with a fixed isometric camera and fixed | ||
| ``800 x 600`` window, and centered automatically by PyVista. |
Comment on lines
+53
to
+56
| @staticmethod | ||
| def VeritySlicerHeartCTData(dirname: Union[str, Path]) -> bool: # noqa: N802 | ||
| """Return True when Slicer-Heart-CT has the expected 4-D CT file.""" | ||
| return DataDownloadTools.VerifySlicerHeartCTData(dirname) |
Comment on lines
+35
to
+44
| REPO_ROOT = Path(__file__).resolve().parent.parent | ||
| TUTORIALS_DIR = Path(__file__).resolve().parent | ||
| DATA_DIR = REPO_ROOT / "data" | ||
| FULL_DATA_DIR = DATA_DIR | ||
| TEST_DATA_DIR = DATA_DIR / "test" | ||
| OUTPUT_DIR = TUTORIALS_DIR / "output" / "tutorial_05" | ||
| BASELINES_DIR = REPO_ROOT / "tests" / "baselines" | ||
| TUTORIAL_02_SURFACE = ( | ||
| TUTORIALS_DIR / "output" / "tutorial_02" / "patient_surfaces.vtp" | ||
| ) |
Comment on lines
+40
to
+46
| REPO_ROOT = Path(__file__).resolve().parent.parent | ||
| TUTORIALS_DIR = Path(__file__).resolve().parent | ||
| DATA_DIR = REPO_ROOT / "data" | ||
| FULL_DATA_DIR = DATA_DIR / "DirLab-4DCT" / "Case1" | ||
| TEST_DATA_DIR = DATA_DIR / "test" / "DirLab-4DCT" / "Case1" | ||
| OUTPUT_DIR = TUTORIALS_DIR / "output" / "tutorial_06" | ||
| BASELINES_DIR = REPO_ROOT / "tests" / "baselines" |
Comment on lines
64
to
69
| # AI/ML and segmentation | ||
| "monai>=1.3.0", | ||
| "nvidia-physicsnemo>=2.0.0,<3.0.0", | ||
| "torch>=2.0.0,<3.0.0", | ||
| "transformers>=4.21.0,<5.0.0", | ||
| "totalsegmentator>=2.0.0", |
Comment on lines
83
to
86
| # Utilities | ||
| "ipykernel>=6.0.0", | ||
| "matplotlib>=3.5.0", | ||
| "typing-extensions>=4.0.0", |
Comment on lines
23
to
27
| "Operating System :: OS Independent", | ||
| "Programming Language :: Python :: 3", | ||
| "Programming Language :: Python :: 3.10", | ||
| "Programming Language :: Python :: 3.11", | ||
| "Programming Language :: Python :: 3.11", | ||
| "Programming Language :: Python :: 3.12", |
Comment on lines
+124
to
+125
| 1. ✅ Install Python 3.10+ and create virtual environment | ||
| 2. ✅ Install development dependencies: ``pip install -e ".[dev]"`` | ||
| 2. ✅ Install development dependencies: ``pip install -e ".[dev]"``, |
Comment on lines
144
to
147
| ## Test-Mode Flag: PHYSIOMOTION_RUNNING_AS_TEST | ||
|
|
||
| When experiment tests run (with `--run-experiments`), the test runner also sets **`PHYSIOMOTION_RUNNING_AS_TEST=1`** so that notebooks can use reduced parameters (e.g. fewer iterations, fewer files) and finish faster. Notebooks should read this variable and choose quick vs full parameters accordingly. See [EXPERIMENT_TESTS_GUIDE.md](EXPERIMENT_TESTS_GUIDE.md#running-as-test-physiomotion_running_as_test) for the recommended check and the `physiomotion4d.notebook_utils.running_as_test()` helper. | ||
| When experiment tests run (with `--run-experiments`), the test runner also sets **`PHYSIOMOTION_RUNNING_AS_TEST=1`** so that notebooks can use reduced parameters (e.g. fewer iterations, fewer files) and finish faster. Notebooks should read this variable and choose quick vs full parameters accordingly. See [EXPERIMENT_TESTS_GUIDE.md](EXPERIMENT_TESTS_GUIDE.md#running-as-test-physiomotion_running_as_test) for the recommended check and the `physiomotion4d.test_tools.TestTools.running_as_test()` helper. | ||
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (16)
src/physiomotion4d/data_download_tools.py (1)
54-56: ⚡ Quick winAdd deprecation warnings to the misspelled compatibility methods.
VeritySlicerHeartCTDataandVerityKCLHeartModelDataare misspelled compatibility shims (should be "Verify"). Consider adding awarnings.warn(DeprecationWarning)and updating their docstrings to indicate they're deprecated in favor of the correctly-spelled methods, so future maintainers and users are aware these will be removed.
⚠️ Proposed deprecation pattern+import warnings + class DataDownloadTools: ... `@staticmethod` def VeritySlicerHeartCTData(dirname: Union[str, Path]) -> bool: # noqa: N802 - """Return True when Slicer-Heart-CT has the expected 4-D CT file.""" + """Deprecated: Use VerifySlicerHeartCTData instead (typo fix).""" + warnings.warn( + "VeritySlicerHeartCTData is deprecated due to typo; " + "use VerifySlicerHeartCTData instead", + DeprecationWarning, + stacklevel=2, + ) return DataDownloadTools.VerifySlicerHeartCTData(dirname)Apply the same pattern to
VerityKCLHeartModelData.Also applies to: 105-107
🤖 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/physiomotion4d/data_download_tools.py` around lines 54 - 56, Add deprecation warnings and updated docstrings to the misspelled compatibility shims: in VeritySlicerHeartCTData and VerityKCLHeartModelData, import the warnings module, emit warnings.warn("Verity... is deprecated; use Verify... instead", DeprecationWarning, stacklevel=2) at the start of each function, update the docstring to indicate deprecation and the correct replacement (VerifySlicerHeartCTData and VerifyKCLHeartModelData), and otherwise keep the function behavior returning DataDownloadTools.VerifySlicerHeartCTData(...) or DataDownloadTools.VerifyKCLHeartModelData(...) respectively so callers still work while getting notified.src/physiomotion4d/anatomy_taxonomy.py (1)
122-147: 💤 Low valueConsider documenting the bounds choice for
range(1, 256).The default
id_range=range(1, 256)implicitly encodes the TotalSegmentator label space (1–255). A short comment or doc reference noting the source of that range, plus the fact that label0(background) is intentionally excluded, would make this easier to revisit when integrating other segmenters with different class index spaces.🤖 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/physiomotion4d/anatomy_taxonomy.py` around lines 122 - 147, The default id_range in fill_other_group currently uses range(1, 256) without explanation; update the function docstring (or add a short inline comment) to state that this choice corresponds to the TotalSegmentator label space (labels 1–255) and that label 0 is intentionally excluded as background, and mention that callers can override id_range for other segmenters; reference the function name fill_other_group, the OTHER_GROUP constant, and the add_organ call so reviewers can find where to update the documentation.src/physiomotion4d/vtk_to_usd/usd_mesh_converter.py (2)
239-253: 💤 Low valueLatent
ValueErrorfromtriangulation_face_map.max()on empty maps.If
triangulate_faceproduces an emptysource_face_index_per_triangle(e.g. a degenerate input where every face has count ≤ 2 contributes nothing),triangulation_face_map.shape[0]is0butarray.data(sized to the source face count) is non-empty. The first branch's equality check isFalse, and the elif evaluatestriangulation_face_map.max()on an empty array, which raisesValueError: zero-size array to reduction operation.This is a degenerate-mesh edge case, but a one-line guard avoids the crash and keeps the existing "fall through and let
create_primvarlog a size mismatch" behavior.🛡️ Proposed guard
if triangulation_face_map is not None and array.interpolation == "uniform": data = np.asarray(array.data) if len(data) == triangulation_face_map.shape[0]: # Already triangle-aligned (e.g. derived primvar that was # built post-triangulation). Leave it alone. pass - elif len(data) > 0 and triangulation_face_map.max() < len(data): + elif ( + len(data) > 0 + and triangulation_face_map.size > 0 + and triangulation_face_map.max() < len(data) + ): expanded_data = data[triangulation_face_map]🤖 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/physiomotion4d/vtk_to_usd/usd_mesh_converter.py` around lines 239 - 253, The branch that checks triangulation_face_map.max() can raise ValueError when triangulation_face_map is empty; update the conditional in the block handling triangulation_face_map (the one that inspects array.interpolation and builds GenericArray) to first ensure triangulation_face_map.size (or shape[0]) > 0 before calling triangulation_face_map.max(), e.g. require triangulation_face_map.size > 0 in the elif so you only call max() when the map is non-empty and preserve the existing fall-through behavior that lets create_primvar handle size mismatches.
386-397: 💤 Low valueReusing cached map is safe here but the contract is implicit.
create_time_varying_meshcorrectly refreshesself._last_triangulation_face_mapvia the innercreate_mesh(first_mesh_data, …)call on line 361, so the cached map matches the topology being authored. The implicit contract — "topology is constant across time samples, therefore the triangulation map computed for sample 0 applies to all samples" — relies on the docstring assumption at line 334. If a future caller violates that assumption (e.g. variable topology meshes), uniform primvars on later samples will be silently misaligned. Consider assertinglen(mesh_data.face_vertex_counts) == len(mesh_data_sequence[0].face_vertex_counts)per iteration to fail fast.🤖 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/physiomotion4d/vtk_to_usd/usd_mesh_converter.py` around lines 386 - 397, The code silently reuses self._last_triangulation_face_map across time samples (created by create_mesh for the first sample) which breaks if topology varies; inside create_time_varying_mesh, before calling _add_generic_arrays for each mesh_data/time_code pair, add a fail-fast assertion that the current mesh_data topology matches the first sample (e.g. assert len(mesh_data.face_vertex_counts) == len(first_mesh_data.face_vertex_counts) or equivalent comparison) so that create_mesh/create_time_varying_mesh will raise immediately if topology changes and avoid misaligned primvars when reusing self._last_triangulation_face_map.src/physiomotion4d/usd_tools.py (3)
132-162: 💤 Low valuePer-face (
uniform)displayColoris silently dropped in favor of the red fallback.When an authored mesh has
displayColorwithinterpolation="uniform"and the color array has face-count entries (≠n_points), the function falls through tolen(colors) != n_points: return fallbackand returns red — visually indistinguishable from "nodisplayColorauthored at all." Meshes authored byapply_colormap_from_primvarand_add_vertex_colorsalways usevertexinterpolation, so this is fine for in-house assets, but third-party USD inputs may trigger it. At minimum, log a warning so the silent red fallback is debuggable.🤖 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/physiomotion4d/usd_tools.py` around lines 132 - 162, _in _openusd_display_color detect when primvar.GetInterpolation() returns UsdGeom.Tokens.uniform (or "uniform") and instead of falling back to red, expand the per-face colors to per-vertex colors by mapping each face's color to its constituent vertices using the mesh topology (use the mesh's GetFaceVertexCounts() and GetFaceVertexIndices() to build a per-vertex array of length n_points), then proceed with the existing tiling/length checks; also add a warning log (e.g., via logging.warning or the module logger) when a uniform interpolation is encountered and converted so third-party USD inputs are debuggable. Ensure you still handle the other branches (constant, single color, vertex-length match) as before and only return the red fallback if conversion or topology lookup fails.
86-90: 💤 Low value
face_counts/face_indiceslookup lacks the same time-sample fallback aspoints.Lines 78–84 correctly fall back to the first authored time sample if
Get(usd_time)returnsNoneforpoints. The topology attributes don't have the same fallback — if a mesh authorsfaceVertexCounts/faceVertexIndicesonly as time samples (atypical, but possible for meshes generated by some tools), the prim is silently skipped instead of recovered. Consider mirroring the points fallback for consistency.🤖 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/physiomotion4d/usd_tools.py` around lines 86 - 90, The code skips meshes when face_counts or face_indices are None but doesn't apply the same time-sample fallback used for points; update the logic around GetFaceVertexCountsAttr() and GetFaceVertexIndicesAttr() to mirror the points fallback: if mesh.GetFaceVertexCountsAttr().Get(usd_time) or mesh.GetFaceVertexIndicesAttr().Get(usd_time) returns None, query the attribute's authored time samples (e.g. via the attr's GetTimeSamples()/GetTimeSamples method or equivalent) and call Get(first_sample) to recover the value before deciding to continue; ensure you reference mesh.GetFaceVertexCountsAttr(), mesh.GetFaceVertexIndicesAttr(), usd_time and use the same fallback pattern already implemented for points.
91-102: ⚖️ Poor tradeoffExtract world matrix once and apply batch transformation to all points instead of per-point Python loop.
The per-point
Gf.Vec3d.Transformloop creates O(N) Python overhead. For medical anatomy meshes with tens of thousands of points, this dominatesload_openusd_as_vtkperformance. While usd-core 23.11 does not expose a dedicated vectorized converter forGf.Matrix4d, you can extract the matrix data usingGetArray()(which returns the 16 doubles in row-major order) and use NumPy's batch matrix-vector multiplication to transform all points in a single operation.🤖 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/physiomotion4d/usd_tools.py` around lines 91 - 102, Replace the per-point Gf.Vec3d.Transform loop in load_openusd_as_vtk: call xform_cache.GetLocalToWorldTransform(prim) once to get world_matrix, extract its flat 16-element array via world_matrix.GetArray(), reshape to a 4x4 NumPy matrix, build a (N,4) homogeneous array from points_value, apply a single NumPy matrix multiplication to transform all points at once, then drop the homogeneous coordinate and cast to float32 into the variable points (instead of calling Gf.Vec3d.Transform inside the list comprehension).src/physiomotion4d/vtk_to_usd/usd_utils.py (1)
432-526: 💤 Low valueConsider making
add_framing_camerafully idempotent for camera attributes.The function correctly reuses the translate op when it already exists (avoiding duplicate ops), but
CreateClippingRangeAttr,CreateFocalLengthAttr,CreateHorizontalApertureAttr, andCreateFocusDistanceAttrare unconditionally set. If these attributes were customized downstream and the function is called again (e.g., during re-export on a stage with a carried-over camera), those tuned values would be silently replaced.All current callers in the repo use default parameters, and no staged USD assets author custom camera attributes, so this is not currently an issue. However, for defensive API design, consider only setting these attributes when not already authored, or expose a
force: bool = Trueparameter to let callers opt out of overwriting.🤖 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/physiomotion4d/vtk_to_usd/usd_utils.py` around lines 432 - 526, The add_framing_camera function currently always overwrites camera attributes (clippingRange, focalLength, horizontalAperture, focusDistance); change it to be idempotent by either adding an optional parameter force: bool = True (defaulting to True to preserve current behavior) or by only setting each attribute when it does not already have an authored value: use camera.GetClippingRangeAttr()/GetFocalLengthAttr()/GetHorizontalApertureAttr()/GetFocusDistanceAttr() and conditionally call Set(...) only if attr.HasAuthoredValue() is False (or force is True), so downstream-customized values are preserved when re-running the function.src/physiomotion4d/transform_tools.py (1)
900-909: ⚡ Quick winExtract stale-layer eviction pattern into a shared helper function.
This same 9-line
unlink()+Sdf.Layer.Find().Clear()+delsequence now appears in four places:transform_tools.py(lines 900–909),usd_tools.py(lines 434–441),converter.py(lines 48–58), andconvert_vtk_to_usd.py(lines 570–577). Pulling it into a small helper invtk_to_usd/usd_utils.py(e.g.evict_stale_usd_layer(output_path: Path) -> None) would centralize the comment explaining USD's global identifier cache and keep all call sites in lockstep if the policy ever changes.🤖 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/physiomotion4d/transform_tools.py` around lines 900 - 909, Extract the 9-line stale-layer eviction pattern (Path.unlink + Sdf.Layer.Find + Clear + del and accompanying comment) into a shared helper named evict_stale_usd_layer(output_path: Path) in vtk_to_usd/usd_utils.py, documenting the USD global identifier cache behavior there; then replace the inline block in transform_tools.py (around the code using output_filename), usd_tools.py, converter.py, and convert_vtk_to_usd.py with a single call to evict_stale_usd_layer(Path(output_filename)) (or the existing Path variable) so all sites use the centralized implementation and comment.src/physiomotion4d/test_tools.py (1)
76-80: ⚡ Quick winInconsistent directory path handling between
baselines_dirandresults_dir.When
baselines_diris provided, line 79 appendsclass_nameto it:baselines_dir = baselines_dir / class_name. However, whenresults_diris provided (line 71), it's used exactly as given without appendingclass_name. This asymmetry can confuse users who expect parallel behavior.Consider either:
- Document this distinction clearly in the docstring (e.g., "
baselines_dir: Root directory;class_namesubdirectory will be created"), or- Make the behavior symmetric by always appending
class_nameto both, or never appending to either when explicitly provided.📝 Proposed docstring clarification
Args: class_name: Subdirectory name for baselines and default results. - results_dir: Root directory for result artifacts. - baselines_dir: Root directory for baseline artifacts. + results_dir: Exact directory for result artifacts when provided; + otherwise defaults to ``tests/results/{class_name}``. + baselines_dir: Root directory for baseline artifacts; + ``class_name`` subdirectory will always be appended. results_output_dir: Optional exact directory for written result🤖 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/physiomotion4d/test_tools.py` around lines 76 - 80, The constructor currently appends class_name to baselines_dir when baselines_dir is passed but does not do the same for results_dir, causing asymmetric behavior; update the logic so results_dir is treated the same: when a results_dir argument is provided, append / class_name before assigning to self._results_dir (mirror the existing baselines_dir behavior using the same pattern as in the baselines block), and update the docstring to state that both baselines_dir and results_dir are root directories where a class_name subdirectory will be used.experiments/Convert_VTK_To_USD/convert_chop_alterra_valve_to_usd.py (1)
169-175: 💤 Low valueHardcoded prim paths may become stale.
Lines 171-173 hardcode specific USD prim paths (
/World/AlterraValve/AlterraValve_object3,/World/AlterraValve/AlterraValve_Triangle) that depend on the VTK file's internal structure. If the Alterra mesh topology or connectivity changes, these paths will break. The computedmesh_pathsat line 169 could be used dynamically to avoid this brittleness.♻️ Consider using dynamic path selection
mesh_paths = usd_tools.list_mesh_paths_under(stage, parent_path="/World/AlterraValve") if separate_by == "connectivity": - vessel_path = "/World/AlterraValve/AlterraValve_object3" + # Pick the largest mesh by vertex count or use a heuristic + vessel_path = mesh_paths[0] if mesh_paths else "/World/AlterraValve/Mesh" elif separate_by == "cell_type": - vessel_path = "/World/AlterraValve/AlterraValve_Triangle" + # Pick triangle mesh or fallback + vessel_path = next((p for p in mesh_paths if "Triangle" in p), mesh_paths[0] if mesh_paths else "/World/AlterraValve/Mesh") else: vessel_path = "/World/AlterraValve/Mesh"🤖 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 `@experiments/Convert_VTK_To_USD/convert_chop_alterra_valve_to_usd.py` around lines 169 - 175, The hardcoded prim paths assigned to vessel_path when handling separate_by ("connectivity" and "cell_type") are brittle; instead, use the computed mesh_paths (from usd_tools.list_mesh_paths_under(stage, parent_path="/World/AlterraValve")) to select the appropriate prim dynamically. Modify the logic around mesh_paths, separate_by and vessel_path: inspect mesh_paths for prim names or metadata that indicate connectivity or cell type (e.g., names containing "object" or "Triangle" or an authored attribute), choose the matching path from mesh_paths for the "connectivity" and "cell_type" branches, and fall back to a default mesh_paths[0] or "/World/AlterraValve/Mesh" if no match is found; keep references to list_mesh_paths_under, mesh_paths, separate_by and vessel_path to locate where to change the code.experiments/Convert_VTK_To_USD/convert_chop_tpv25_valve_to_usd.py (1)
166-172: 💤 Low valueHardcoded vessel paths may be fragile.
The script now uses fixed paths like
"/World/TPV25Valve/TPV25Valve_object4"and"/World/TPV25Valve/TPV25Valve_Triangle"for specificseparate_bymodes. If the VTK file's connectivity structure or cell-type composition changes, these paths will break silently. Additionally,mesh_pathsis computed but unused in these branches.Consider either:
- Adding a comment explaining why
object4andTriangleare the correct choices for this specific dataset, or- Selecting from
mesh_pathsusing a heuristic (e.g., largest component, specific substring match) to make the script more robust.🤖 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 `@experiments/Convert_VTK_To_USD/convert_chop_tpv25_valve_to_usd.py` around lines 166 - 172, The hardcoded vessel_path assignments for separate_by modes (the separate_by variable and vessel_path binding) are fragile and ignore the previously computed mesh_paths from usd_tools.list_mesh_paths_under; replace the fixed literals with a robust selection: inspect mesh_paths (returned by usd_tools.list_mesh_paths_under(stage, parent_path="/World/TPV25Valve")) and choose the best match for vessel_path via a heuristic (e.g., prefer paths containing "object" or "Triangle" when separate_by == "connectivity" or "cell_type", otherwise pick the largest/first mesh path), and fall back to "/World/TPV25Valve/Mesh" only if no candidate found; keep the selection logic encapsulated near where separate_by and vessel_path are set so callers using vessel_path get a resilient value.tests/test_vtk_to_usd_library.py (1)
127-154: ⚡ Quick winConsider adding a clarifying comment about the default color behavior.
The assertion that
openusd_rgbequals[255, 0, 0]is correct—load_openusd_as_vtkapplies red as the default color when geometry lacks adisplayColorprimvar (as documented in the function's docstring). Adding a comment in the test linking to this documented behavior would improve clarity for future readers.🤖 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 `@tests/test_vtk_to_usd_library.py` around lines 127 - 154, Add a short clarifying comment inside the test_openusd_screenshot_uses_vtk_loader test explaining why the expected color is red; mention that load_openusd_as_vtk applies a default red color when geometry lacks a displayColor primvar (per load_openusd_as_vtk docstring) so the assertion on point_data["openusd_rgb"] == [255, 0, 0] is intentional and not test-data-specific.src/physiomotion4d/workflow_convert_heart_gated_ct_to_usd.py (1)
145-176: 💤 Low valueConsider consistent parameter signature for conditional write helpers.
The helpers have slightly inconsistent signatures:
_write_image_if_enabledtakes anenabledparameter_write_transform_if_enabledreadsself.save_registration_transformsdirectlyThis is not incorrect, but for maintainability, consider making both take an
enabledparameter for consistency. This would make the helpers more reusable if additional conditional write scenarios arise.♻️ Possible refactor for consistency
def _write_transform_if_enabled( self, transform: itk.Transform, filename: str, + enabled: bool, ) -> None: """Write a transform artifact when transform saving is enabled.""" - if self.save_registration_transforms: + if enabled: itk.transformwrite( transform, self._output_path(filename), compression=True, )Then update call sites to pass
self.save_registration_transforms.🤖 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/physiomotion4d/workflow_convert_heart_gated_ct_to_usd.py` around lines 145 - 176, Make the three conditional write helpers consistent by adding an enabled: bool parameter to _write_transform_if_enabled and _write_registered_image_if_enabled (like _write_image_if_enabled) and use that parameter to gate writes instead of reading instance flags internally; update all call sites to pass the appropriate flags (pass self.save_registration_transforms to _write_transform_if_enabled and self.save_registered_images to _write_registered_image_if_enabled) and keep the same behavior for writing (itk.transformwrite, itk.imwrite, and registrar.get_registered_image()).tutorials/tutorial_04_fit_statistical_model_to_patient.py (1)
89-100: 💤 Low valueConsider extracting duplicate surface extraction logic.
The surface extraction pattern (lines 70-76 and 92-99) is duplicated. For maintainability, consider extracting to a helper function like
_extract_surface(data: pv.DataSet) -> pv.PolyData.♻️ Optional refactor
def _extract_surface(data: pv.DataSet) -> pv.PolyData: """Extract PolyData surface from any PyVista dataset.""" if isinstance(data, pv.PolyData): return data return cast( pv.PolyData, data.extract_surface(algorithm="dataset_surface"), ) # Usage: template_model = _extract_surface(cast(pv.DataSet, pv.read(str(template_file))))🤖 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 89 - 100, Duplicate surface-extraction logic for template and patient samples should be moved into a helper function (e.g., _extract_surface(data: pv.DataSet) -> pv.PolyData) and then used from both places; implement _extract_surface to return the input if it's already a pv.PolyData otherwise call data.extract_surface(algorithm="dataset_surface") and cast to pv.PolyData, then replace the inline checks in the template_model creation and the patient_models loop (references: template_model, patient_models, sample_files) to call _extract_surface(cast(pv.DataSet, pv.read(...))) or _extract_surface(sample_data).tests/test_tutorials.py (1)
65-70: 💤 Low valueImprove error message for missing
tutorial_results.If a tutorial script crashes or fails to set
tutorial_results, the assertion on line 69 could be more informative by checking for None first.♻️ Optional improvement
def _run_tutorial_script(script_name: str) -> dict[str, Any]: """Run a tutorial script with no command-line arguments.""" namespace = runpy.run_path(str(_REPO_ROOT / "tutorials" / script_name)) results = namespace.get("tutorial_results") + if results is None: + pytest.fail( + f"{script_name} did not set tutorial_results. " + "Script may have crashed or exited early." + ) assert isinstance(results, dict), f"{script_name} did not set tutorial_results" return results🤖 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 `@tests/test_tutorials.py` around lines 65 - 70, The assertion for tutorial_results should first check for None and raise a clearer error; in _run_tutorial_script capture the namespace from runpy.run_path, then if namespace.get("tutorial_results") is None raise an AssertionError that includes script_name and the namespace keys (e.g. f"{script_name} did not set tutorial_results; namespace keys: {list(namespace.keys())}"), and only afterward assert isinstance(results, dict) to ensure it's the expected type; update _run_tutorial_script accordingly (function name: _run_tutorial_script, variable: tutorial_results).
🤖 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 @.agents/skills/skills/caveman/SKILL.md:
- Line 7: The SKILL.md metadata currently publishes a personal email in the
author field; update the author line (the "author:" metadata in SKILL.md) to
remove the personal email and replace it with a non-PII alternative such as a
team alias (e.g., "NVIDIA ML Team"), a role-based address, or a profile URL
(e.g., a GitHub/GitLab profile) while preserving attribution to Vaibhav Phutane
and Julius Brussee.
In `@docs/api/cli/index.rst`:
- Around line 9-11: Replace the incorrect article before the inline code
reference: change the phrase "an `main()` function" to "a `main()` function" in
the CLI documentation sentence that describes each module exposing a main() that
parses argparse and dispatches to the workflow class; update the inline text
near the existing `main()` mention so the sentence reads "a `main()` function".
In `@experiments/Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.py`:
- Around line 23-26: The copy operation uses the wrong source path: after
conv.save_3d_images writes files into data_dir / "slice", shutil.copyfile is
trying to read data_dir / "slice_007.mha" (parent dir) which causes
FileNotFoundError; update the shutil.copyfile source to point to the generated
file inside the slice subdirectory (e.g. data_dir / "slice" / "slice_007.mha")
when copying to output_dir / "slice_fixed.mha", ensuring you join Paths
consistently with the rest of the script and convert to str if required by
shutil.copyfile.
In `@experiments/Heart-GatedCT_To_USD/1-register_images.py`:
- Line 84: The loop currently hardcodes a single slice with for i in range(17,
18, FRAME_STEP) which disables multi-frame processing; change that range to
iterate the full set of frames (e.g., for i in range(0, total_slices,
FRAME_STEP) or for i in range(0, len(slices), FRAME_STEP)) so the variable i and
FRAME_STEP drive processing over all slices rather than just slice 017.
In `@experiments/Heart-GatedCT_To_USD/3-transform_dynamic_and_static_contours.py`:
- Around line 14-20: The test_mode check is defined at module import time
(test_mode = TestTools.running_as_test()) which contradicts the defensive
comment about guarding side-effects; move the TestTools.running_as_test() call
and the test_mode variable assignment inside the if __name__ == "__main__":
block (before any use of seg or any potential seg.segment(...) calls) so the
check runs only when the script is executed as main and not at import time;
update any downstream uses to reference the local test_mode defined inside that
main guard.
In `@pyproject.toml`:
- Around line 25-26: Remove the duplicate Python classifier entry in
pyproject.toml: locate the repeated string "Programming Language :: Python ::
3.11" and delete one of the two identical lines so the classifiers list contains
only a single Python 3.11 entry.
In `@src/physiomotion4d/anatomy_taxonomy.py`:
- Around line 71-85: Docstring for add_organ states cross-group id collisions
should be last-write-wins but group_for_id and group_for_label currently iterate
self._groups.values() in insertion order and therefore return the first match;
choose one fix: (A) update add_organ docstring to state it is first-write-wins,
or (B) change group_for_id and group_for_label to iterate groups in reverse
insertion order (e.g. iterate reversed(list(self._groups.values())) or reversed
tuple of keys) so the most recently added group wins; update unit tests/comments
accordingly to reflect the chosen semantics.
In `@src/physiomotion4d/segment_chest_total_segmentator.py`:
- Around line 277-284: The code currently does mask3 = (labelmap_arr3 == 1)
which collapses all classes into a binary mask so subsequent assignments for IDs
121/122/123 never run; change the logic to preserve per-class masks from
labelmap_arr3 (e.g. compute separate boolean masks labelmap_arr3 == 1,
labelmap_arr3 == 2, labelmap_arr3 == 3, labelmap_arr3 == 4, convert each to
uint8/ITK image and CopyInformation from preprocessed_image as you do now) and
then set final_arr using those per-class masks (using the same final_arr =
np.where(..., value, final_arr) pattern) so classes 121/122/123 are correctly
applied.
In `@src/physiomotion4d/segment_heart_simpleware.py`:
- Around line 293-297: The code calls next(fh) to skip the CSV header but
doesn't guard against an empty/truncated landmarks_file which raises
StopIteration; modify the block around landmarks_file/next(fh)/csv.DictReader to
first check that the file has content (e.g., read the first line via
fh.readline() and only call csv.DictReader on the remainder if that line is
non-empty) or wrap next(fh) in a try/except StopIteration and treat the file as
empty (skip processing) so the segmentation continues; update the logic
referencing landmarks_file, next(fh) and csv.DictReader accordingly.
In `@src/physiomotion4d/usd_anatomy_tools.py`:
- Around line 391-402: The loop over stage primitives uses UsdGeom.Mesh(prim)
and a truthiness check which doesn't filter non-mesh prims; change the logic to
first test the prim type (e.g., use prim.IsA(UsdGeom.Mesh)) or construct
mesh_prim and call mesh_prim.IsDefined() before proceeding, then only look up
organ_params and call apply_anatomy_material_to_prim(prim, params) for valid
mesh prims; update the block around Traverse(), UsdGeom.Mesh,
prim.IsA/IsDefined, organ_params, and apply_anatomy_material_to_prim
accordingly.
In `@src/physiomotion4d/vtk_to_usd/usd_utils.py`:
- Around line 470-483: The camera auto-positioning path that computes bounds
when bounds_min/bounds_max are None currently always offsets the camera along
+Z; change it to query the stage up axis via UsdGeom.GetStageUpAxis(stage) and
choose an offset axis perpendicular to that up axis (e.g., if up is 'Z' offset
along +Y, otherwise offset along +Z) when computing the camera translation from
center and distance; update the code that builds the camera position
(referencing stage, bounds_min, bounds_max, center, distance) to use this
computed offset vector so Z-up stages are framed correctly.
In `@tutorials/tutorial_01_heart_gated_ct_to_usd.py`:
- Around line 100-113: The code is forcing test mode by overwriting
TestTools.running_as_test() with a hardcoded assignment; remove the stray
assignment "test_mode = True" so that test_mode remains set by
TestTools.running_as_test(), allowing data_dir selection (TEST_DATA_DIR vs
FULL_DATA_DIR) and number_of_registration_iterations to behave correctly (1 when
test_mode True, 10 when False); ensure no other parts of the file reassign
test_mode and that subsequent logic uses this single variable (refer to
TestTools.running_as_test(), test_mode, data_dir, and
number_of_registration_iterations).
In `@tutorials/tutorial_03_create_statistical_model.py`:
- Around line 110-111: The JSON dump fails because pca_model contains NumPy
ndarrays; before calling json.dump on model_file, convert NumPy arrays (e.g.,
pca_model["components"] and pca_model["eigenvalues"] or any ndarray values in
pca_model) to native Python types (use .tolist() or recursively map ndarray ->
list), then write the converted dict to disk; you can create a small sanitized
copy (e.g., pca_model_serializable) by checking isinstance(value, numpy.ndarray)
and converting, and pass that to json.dump.
In `@tutorials/tutorial_04_fit_statistical_model_to_patient.py`:
- Around line 69-76: The call to pv.DataSet.extract_surface uses the unsupported
algorithm="dataset_surface" parameter (seen where template_data is converted to
template_model); remove the algorithm="dataset_surface" argument and call
extract_surface() with no parameters (or else upgrade PyVista to >=0.47.0 if you
need that option); search for other uses of extract_surface in this file and
remove the algorithm=... parameter there as well to ensure compatibility with
PyVista 0.43.0.
In `@tutorials/tutorial_08_dirlab_pca_time_series.py`:
- Around line 137-141: The loop using zip over phase_files,
registration_result["forward_transforms"], and
registration_result["inverse_transforms"] can silently drop entries when lengths
differ; before iterating, validate that len(phase_files) equals
len(registration_result["forward_transforms"]) and equals
len(registration_result["inverse_transforms"]) and raise/log a clear error (or
handle the mismatch) if not; update the iteration around the for phase_file,
phase_to_reference, reference_to_phase in zip(...) to either use an explicit
zip_longest with a clear check or bail out with a descriptive exception
mentioning phase_files and the forward_transforms/inverse_transforms keys so
mismatches are detected instead of truncated.
---
Nitpick comments:
In `@experiments/Convert_VTK_To_USD/convert_chop_alterra_valve_to_usd.py`:
- Around line 169-175: The hardcoded prim paths assigned to vessel_path when
handling separate_by ("connectivity" and "cell_type") are brittle; instead, use
the computed mesh_paths (from usd_tools.list_mesh_paths_under(stage,
parent_path="/World/AlterraValve")) to select the appropriate prim dynamically.
Modify the logic around mesh_paths, separate_by and vessel_path: inspect
mesh_paths for prim names or metadata that indicate connectivity or cell type
(e.g., names containing "object" or "Triangle" or an authored attribute), choose
the matching path from mesh_paths for the "connectivity" and "cell_type"
branches, and fall back to a default mesh_paths[0] or "/World/AlterraValve/Mesh"
if no match is found; keep references to list_mesh_paths_under, mesh_paths,
separate_by and vessel_path to locate where to change the code.
In `@experiments/Convert_VTK_To_USD/convert_chop_tpv25_valve_to_usd.py`:
- Around line 166-172: The hardcoded vessel_path assignments for separate_by
modes (the separate_by variable and vessel_path binding) are fragile and ignore
the previously computed mesh_paths from usd_tools.list_mesh_paths_under; replace
the fixed literals with a robust selection: inspect mesh_paths (returned by
usd_tools.list_mesh_paths_under(stage, parent_path="/World/TPV25Valve")) and
choose the best match for vessel_path via a heuristic (e.g., prefer paths
containing "object" or "Triangle" when separate_by == "connectivity" or
"cell_type", otherwise pick the largest/first mesh path), and fall back to
"/World/TPV25Valve/Mesh" only if no candidate found; keep the selection logic
encapsulated near where separate_by and vessel_path are set so callers using
vessel_path get a resilient value.
In `@src/physiomotion4d/anatomy_taxonomy.py`:
- Around line 122-147: The default id_range in fill_other_group currently uses
range(1, 256) without explanation; update the function docstring (or add a short
inline comment) to state that this choice corresponds to the TotalSegmentator
label space (labels 1–255) and that label 0 is intentionally excluded as
background, and mention that callers can override id_range for other segmenters;
reference the function name fill_other_group, the OTHER_GROUP constant, and the
add_organ call so reviewers can find where to update the documentation.
In `@src/physiomotion4d/data_download_tools.py`:
- Around line 54-56: Add deprecation warnings and updated docstrings to the
misspelled compatibility shims: in VeritySlicerHeartCTData and
VerityKCLHeartModelData, import the warnings module, emit
warnings.warn("Verity... is deprecated; use Verify... instead",
DeprecationWarning, stacklevel=2) at the start of each function, update the
docstring to indicate deprecation and the correct replacement
(VerifySlicerHeartCTData and VerifyKCLHeartModelData), and otherwise keep the
function behavior returning DataDownloadTools.VerifySlicerHeartCTData(...) or
DataDownloadTools.VerifyKCLHeartModelData(...) respectively so callers still
work while getting notified.
In `@src/physiomotion4d/test_tools.py`:
- Around line 76-80: The constructor currently appends class_name to
baselines_dir when baselines_dir is passed but does not do the same for
results_dir, causing asymmetric behavior; update the logic so results_dir is
treated the same: when a results_dir argument is provided, append / class_name
before assigning to self._results_dir (mirror the existing baselines_dir
behavior using the same pattern as in the baselines block), and update the
docstring to state that both baselines_dir and results_dir are root directories
where a class_name subdirectory will be used.
In `@src/physiomotion4d/transform_tools.py`:
- Around line 900-909: Extract the 9-line stale-layer eviction pattern
(Path.unlink + Sdf.Layer.Find + Clear + del and accompanying comment) into a
shared helper named evict_stale_usd_layer(output_path: Path) in
vtk_to_usd/usd_utils.py, documenting the USD global identifier cache behavior
there; then replace the inline block in transform_tools.py (around the code
using output_filename), usd_tools.py, converter.py, and convert_vtk_to_usd.py
with a single call to evict_stale_usd_layer(Path(output_filename)) (or the
existing Path variable) so all sites use the centralized implementation and
comment.
In `@src/physiomotion4d/usd_tools.py`:
- Around line 132-162: _in _openusd_display_color detect when
primvar.GetInterpolation() returns UsdGeom.Tokens.uniform (or "uniform") and
instead of falling back to red, expand the per-face colors to per-vertex colors
by mapping each face's color to its constituent vertices using the mesh topology
(use the mesh's GetFaceVertexCounts() and GetFaceVertexIndices() to build a
per-vertex array of length n_points), then proceed with the existing
tiling/length checks; also add a warning log (e.g., via logging.warning or the
module logger) when a uniform interpolation is encountered and converted so
third-party USD inputs are debuggable. Ensure you still handle the other
branches (constant, single color, vertex-length match) as before and only return
the red fallback if conversion or topology lookup fails.
- Around line 86-90: The code skips meshes when face_counts or face_indices are
None but doesn't apply the same time-sample fallback used for points; update the
logic around GetFaceVertexCountsAttr() and GetFaceVertexIndicesAttr() to mirror
the points fallback: if mesh.GetFaceVertexCountsAttr().Get(usd_time) or
mesh.GetFaceVertexIndicesAttr().Get(usd_time) returns None, query the
attribute's authored time samples (e.g. via the attr's
GetTimeSamples()/GetTimeSamples method or equivalent) and call Get(first_sample)
to recover the value before deciding to continue; ensure you reference
mesh.GetFaceVertexCountsAttr(), mesh.GetFaceVertexIndicesAttr(), usd_time and
use the same fallback pattern already implemented for points.
- Around line 91-102: Replace the per-point Gf.Vec3d.Transform loop in
load_openusd_as_vtk: call xform_cache.GetLocalToWorldTransform(prim) once to get
world_matrix, extract its flat 16-element array via world_matrix.GetArray(),
reshape to a 4x4 NumPy matrix, build a (N,4) homogeneous array from
points_value, apply a single NumPy matrix multiplication to transform all points
at once, then drop the homogeneous coordinate and cast to float32 into the
variable points (instead of calling Gf.Vec3d.Transform inside the list
comprehension).
In `@src/physiomotion4d/vtk_to_usd/usd_mesh_converter.py`:
- Around line 239-253: The branch that checks triangulation_face_map.max() can
raise ValueError when triangulation_face_map is empty; update the conditional in
the block handling triangulation_face_map (the one that inspects
array.interpolation and builds GenericArray) to first ensure
triangulation_face_map.size (or shape[0]) > 0 before calling
triangulation_face_map.max(), e.g. require triangulation_face_map.size > 0 in
the elif so you only call max() when the map is non-empty and preserve the
existing fall-through behavior that lets create_primvar handle size mismatches.
- Around line 386-397: The code silently reuses
self._last_triangulation_face_map across time samples (created by create_mesh
for the first sample) which breaks if topology varies; inside
create_time_varying_mesh, before calling _add_generic_arrays for each
mesh_data/time_code pair, add a fail-fast assertion that the current mesh_data
topology matches the first sample (e.g. assert len(mesh_data.face_vertex_counts)
== len(first_mesh_data.face_vertex_counts) or equivalent comparison) so that
create_mesh/create_time_varying_mesh will raise immediately if topology changes
and avoid misaligned primvars when reusing self._last_triangulation_face_map.
In `@src/physiomotion4d/vtk_to_usd/usd_utils.py`:
- Around line 432-526: The add_framing_camera function currently always
overwrites camera attributes (clippingRange, focalLength, horizontalAperture,
focusDistance); change it to be idempotent by either adding an optional
parameter force: bool = True (defaulting to True to preserve current behavior)
or by only setting each attribute when it does not already have an authored
value: use
camera.GetClippingRangeAttr()/GetFocalLengthAttr()/GetHorizontalApertureAttr()/GetFocusDistanceAttr()
and conditionally call Set(...) only if attr.HasAuthoredValue() is False (or
force is True), so downstream-customized values are preserved when re-running
the function.
In `@src/physiomotion4d/workflow_convert_heart_gated_ct_to_usd.py`:
- Around line 145-176: Make the three conditional write helpers consistent by
adding an enabled: bool parameter to _write_transform_if_enabled and
_write_registered_image_if_enabled (like _write_image_if_enabled) and use that
parameter to gate writes instead of reading instance flags internally; update
all call sites to pass the appropriate flags (pass
self.save_registration_transforms to _write_transform_if_enabled and
self.save_registered_images to _write_registered_image_if_enabled) and keep the
same behavior for writing (itk.transformwrite, itk.imwrite, and
registrar.get_registered_image()).
In `@tests/test_tutorials.py`:
- Around line 65-70: The assertion for tutorial_results should first check for
None and raise a clearer error; in _run_tutorial_script capture the namespace
from runpy.run_path, then if namespace.get("tutorial_results") is None raise an
AssertionError that includes script_name and the namespace keys (e.g.
f"{script_name} did not set tutorial_results; namespace keys:
{list(namespace.keys())}"), and only afterward assert isinstance(results, dict)
to ensure it's the expected type; update _run_tutorial_script accordingly
(function name: _run_tutorial_script, variable: tutorial_results).
In `@tests/test_vtk_to_usd_library.py`:
- Around line 127-154: Add a short clarifying comment inside the
test_openusd_screenshot_uses_vtk_loader test explaining why the expected color
is red; mention that load_openusd_as_vtk applies a default red color when
geometry lacks a displayColor primvar (per load_openusd_as_vtk docstring) so the
assertion on point_data["openusd_rgb"] == [255, 0, 0] is intentional and not
test-data-specific.
In `@tutorials/tutorial_04_fit_statistical_model_to_patient.py`:
- Around line 89-100: Duplicate surface-extraction logic for template and
patient samples should be moved into a helper function (e.g.,
_extract_surface(data: pv.DataSet) -> pv.PolyData) and then used from both
places; implement _extract_surface to return the input if it's already a
pv.PolyData otherwise call data.extract_surface(algorithm="dataset_surface") and
cast to pv.PolyData, then replace the inline checks in the template_model
creation and the patient_models loop (references: template_model,
patient_models, sample_files) to call _extract_surface(cast(pv.DataSet,
pv.read(...))) or _extract_surface(sample_data).
🪄 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: 0456f41b-43eb-4c42-be07-e525c2edc19e
⛔ Files ignored due to path filters (6)
tests/baselines/tutorial_01_heart_gated_ct_to_usd/contours_3d.pngis excluded by!**/*.pngtests/baselines/tutorial_01_heart_gated_ct_to_usd/reference_frame_axial.pngis excluded by!**/*.pngtests/baselines/tutorial_01_heart_gated_ct_to_usd/segmentation_overlay.pngis excluded by!**/*.pngtests/baselines/tutorial_02_ct_to_vtk/segmentation_overlay.pngis excluded by!**/*.pngtests/baselines/tutorial_02_ct_to_vtk/vtk_surfaces.pngis excluded by!**/*.pngtests/baselines/tutorial_05_vtk_to_usd/usd_mesh_rendering.pngis excluded by!**/*.png
📒 Files selected for processing (117)
.agents/.gitignore.agents/skills/skills/caveman/SKILL.mdREADME.mddata/README.mddata/Slicer-Heart-CT/download_and_convert.pydata/test/.gitignoredocs/API_MAP.mddocs/api/base.rstdocs/api/cli/convert_ct_to_vtk.rstdocs/api/cli/convert_heart_gated_ct_to_usd.rstdocs/api/cli/convert_vtk_to_usd.rstdocs/api/cli/create_statistical_model.rstdocs/api/cli/fit_statistical_model_to_patient.rstdocs/api/cli/index.rstdocs/api/cli/reconstruct_highres_4d_ct.rstdocs/api/cli/visualize_pca_modes.rstdocs/api/index.rstdocs/api/model_registration/distance_maps.rstdocs/api/model_registration/icp.rstdocs/api/model_registration/icp_itk.rstdocs/api/model_registration/pca.rstdocs/api/registration/ants.rstdocs/api/registration/base.rstdocs/api/registration/greedy.rstdocs/api/registration/icon.rstdocs/api/registration/index.rstdocs/api/registration/time_series.rstdocs/api/segmentation/base.rstdocs/api/segmentation/index.rstdocs/api/segmentation/simpleware.rstdocs/api/segmentation/totalsegmentator.rstdocs/api/usd/index.rstdocs/api/usd/vtk_conversion.rstdocs/api/usd/vtk_to_usd_lib.rstdocs/api/utilities/contour_tools.rstdocs/api/utilities/data_download.rstdocs/api/utilities/index.rstdocs/api/utilities/test_tools.rstdocs/api/workflows.rstdocs/contributing.rstdocs/developer/segmentation.rstdocs/developer/usd_generation.rstdocs/examples.rstdocs/quickstart.rstdocs/tutorials.rstexperiments/Colormap-VTK_To_USD/colormap_vtk_to_usd.pyexperiments/Convert_VTK_To_USD/convert_chop_alterra_valve_to_usd.pyexperiments/Convert_VTK_To_USD/convert_chop_tpv25_valve_to_usd.pyexperiments/Heart-Create_Statistical_Model/1-input_meshes_to_input_surfaces.pyexperiments/Heart-Create_Statistical_Model/2-input_surfaces_to_surfaces_aligned.pyexperiments/Heart-Create_Statistical_Model/3-registration_based_correspondence.pyexperiments/Heart-Create_Statistical_Model/4-surfaces_aligned_correspond_to_pca_inputs.pyexperiments/Heart-Create_Statistical_Model/5-compute_pca_model.pyexperiments/Heart-Create_Statistical_Model/README.mdexperiments/Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.pyexperiments/Heart-GatedCT_To_USD/1-register_images.pyexperiments/Heart-GatedCT_To_USD/2-generate_segmentation.pyexperiments/Heart-GatedCT_To_USD/3-transform_dynamic_and_static_contours.pyexperiments/Heart-GatedCT_To_USD/test_compare_registration_speed.pyexperiments/Heart-Simpleware_Segmentation/README.mdexperiments/Heart-Simpleware_Segmentation/simpleware_heart_segmentation.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_icp_itk.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_model_registration_pca.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient-CHOPValve.pyexperiments/Heart-Statistical_Model_To_Patient/heart_model_to_patient.pyexperiments/Heart-VTKSeries_To_USD/0-download_and_convert_4d_to_3d.pyexperiments/Heart-VTKSeries_To_USD/1-heart_vtkseries_to_usd.pyexperiments/Lung-GatedCT_To_USD/0-register_dirlab_4dct.pyexperiments/Lung-GatedCT_To_USD/1-make_dirlab_models.pyexperiments/Lung-GatedCT_To_USD/2-paint_dirlab_models.pyexperiments/Lung-GatedCT_To_USD/Experiment_SegReg.pyexperiments/README.mdexperiments/Reconstruct4DCT/reconstruct_4d_ct_class.pypyproject.tomlsrc/physiomotion4d/__init__.pysrc/physiomotion4d/anatomy_taxonomy.pysrc/physiomotion4d/cli/convert_vtk_to_usd.pysrc/physiomotion4d/convert_vtk_to_usd.pysrc/physiomotion4d/data_download_tools.pysrc/physiomotion4d/notebook_utils.pysrc/physiomotion4d/register_images_ants.pysrc/physiomotion4d/segment_anatomy_base.pysrc/physiomotion4d/segment_chest_total_segmentator.pysrc/physiomotion4d/segment_heart_simpleware.pysrc/physiomotion4d/test_tools.pysrc/physiomotion4d/transform_tools.pysrc/physiomotion4d/usd_anatomy_tools.pysrc/physiomotion4d/usd_tools.pysrc/physiomotion4d/vtk_to_usd/__init__.pysrc/physiomotion4d/vtk_to_usd/converter.pysrc/physiomotion4d/vtk_to_usd/material_manager.pysrc/physiomotion4d/vtk_to_usd/primvar_derivations.pysrc/physiomotion4d/vtk_to_usd/usd_mesh_converter.pysrc/physiomotion4d/vtk_to_usd/usd_utils.pysrc/physiomotion4d/workflow_convert_ct_to_vtk.pysrc/physiomotion4d/workflow_convert_heart_gated_ct_to_usd.pytests/EXPERIMENT_FLAG_USAGE.mdtests/EXPERIMENT_TESTS_GUIDE.mdtests/EXPERIMENT_TESTS_SUMMARY.mdtests/PARALLEL_EXECUTION_GUIDE.mdtests/conftest.pytests/test_anatomy_taxonomy.pytests/test_convert_vtk_to_usd.pytests/test_download_heart_data.pytests/test_segment_chest_total_segmentator.pytests/test_segment_heart_simpleware.pytests/test_tutorials.pytests/test_vtk_to_usd_library.pytutorials/README.mdtutorials/tutorial_01_heart_gated_ct_to_usd.pytutorials/tutorial_02_ct_to_vtk.pytutorials/tutorial_03_create_statistical_model.pytutorials/tutorial_04_fit_statistical_model_to_patient.pytutorials/tutorial_05_vtk_to_usd.pytutorials/tutorial_06_reconstruct_highres_4d_ct.pytutorials/tutorial_07_dirlab_pca_model.pytutorials/tutorial_08_dirlab_pca_time_series.py
💤 Files with no reviewable changes (1)
- src/physiomotion4d/notebook_utils.py
Comment on lines
+23
to
+26
| conv.save_3d_images(str(data_dir / "slice")) | ||
|
|
||
| # Save the mid-stroke slice as the fixed/reference image | ||
| shutil.copyfile(f"{data_dir}/slice_007.mha", f"{output_dir}/slice_fixed.mha") | ||
| shutil.copyfile(data_dir / "slice_007.mha", output_dir / "slice_fixed.mha") |
There was a problem hiding this comment.
Copy source path missing the slice/ subdirectory will raise FileNotFoundError.
Line 23 writes the per-frame 3D outputs under data_dir / "slice" (e.g. …/Slicer-Heart-CT/slice/slice_007.mha), but line 26 copies from data_dir / "slice_007.mha" — the parent directory. The AI summary also describes this exact path (data_dir / "slice" / "slice_007.mha") as the intended source, so the diff appears to have dropped the subdirectory.
🐛 Proposed fix
# Save the mid-stroke slice as the fixed/reference image
-shutil.copyfile(data_dir / "slice_007.mha", output_dir / "slice_fixed.mha")
+shutil.copyfile(data_dir / "slice" / "slice_007.mha", output_dir / "slice_fixed.mha")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| conv.save_3d_images(str(data_dir / "slice")) | |
| # Save the mid-stroke slice as the fixed/reference image | |
| shutil.copyfile(f"{data_dir}/slice_007.mha", f"{output_dir}/slice_fixed.mha") | |
| shutil.copyfile(data_dir / "slice_007.mha", output_dir / "slice_fixed.mha") | |
| conv.save_3d_images(str(data_dir / "slice")) | |
| # Save the mid-stroke slice as the fixed/reference image | |
| shutil.copyfile(data_dir / "slice" / "slice_007.mha", output_dir / "slice_fixed.mha") |
🤖 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 `@experiments/Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.py` around
lines 23 - 26, The copy operation uses the wrong source path: after
conv.save_3d_images writes files into data_dir / "slice", shutil.copyfile is
trying to read data_dir / "slice_007.mha" (parent dir) which causes
FileNotFoundError; update the shutil.copyfile source to point to the generated
file inside the slice subdirectory (e.g. data_dir / "slice" / "slice_007.mha")
when copying to output_dir / "slice_fixed.mha", ensuring you join Paths
consistently with the rest of the script and convert to str if required by
shutil.copyfile.
Comment on lines
+110
to
+111
| with model_file.open("w", encoding="utf-8") as f: | ||
| json.dump(pca_model, f, indent=2) |
There was a problem hiding this comment.
JSON serialization will fail with NumPy arrays.
The workflow likely returns pca_model with NumPy array values for "components" and "eigenvalues". Calling json.dump(pca_model, f) directly will raise TypeError: Object of type ndarray is not JSON serializable.
🐛 Proposed fix: Convert arrays before serialization
model_file = output_dir / "pca_model.json"
+
+# Convert numpy arrays to lists for JSON serialization
+pca_model_serializable = {
+ k: v.tolist() if hasattr(v, "tolist") else v
+ for k, v in pca_model.items()
+}
+
with model_file.open("w", encoding="utf-8") as f:
- json.dump(pca_model, f, indent=2)
+ json.dump(pca_model_serializable, f, indent=2)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| with model_file.open("w", encoding="utf-8") as f: | |
| json.dump(pca_model, f, indent=2) | |
| model_file = output_dir / "pca_model.json" | |
| # Convert numpy arrays to lists for JSON serialization | |
| pca_model_serializable = { | |
| k: v.tolist() if hasattr(v, "tolist") else v | |
| for k, v in pca_model.items() | |
| } | |
| with model_file.open("w", encoding="utf-8") as f: | |
| json.dump(pca_model_serializable, f, indent=2) |
🤖 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_03_create_statistical_model.py` around lines 110 - 111,
The JSON dump fails because pca_model contains NumPy ndarrays; before calling
json.dump on model_file, convert NumPy arrays (e.g., pca_model["components"] and
pca_model["eigenvalues"] or any ndarray values in pca_model) to native Python
types (use .tolist() or recursively map ndarray -> list), then write the
converted dict to disk; you can create a small sanitized copy (e.g.,
pca_model_serializable) by checking isinstance(value, numpy.ndarray) and
converting, and pass that to json.dump.
Comment on lines
+69
to
+76
| template_data = cast(pv.DataSet, pv.read(str(template_file))) | ||
| if isinstance(template_data, pv.PolyData): | ||
| template_model = template_data | ||
| else: | ||
| template_model = cast( | ||
| pv.PolyData, | ||
| template_data.extract_surface(algorithm="dataset_surface"), | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PyVista 0.43.0 extract_surface algorithm parameter documentation
💡 Result:
The extract_surface method in PyVista 0.43.0 does not have an 'algorithm' parameter. This parameter was added in a later version (around 0.47.0), as evidenced by its absence in the 0.43.1 example and docs [1][2], and presence in 0.47+ docs [3][4][5]. In PyVista 0.43.0 (and 0.42.3 docs), the signature is: DataSetFilters.extract_surface(pass_pointid=True, pass_cellid=True, nonlinear_subdivision=1, progress_bar=False) -> PolyData [2] Parameters: - pass_pointid: bool, default True. Adds point array "vtkOriginalPointIds" identifying original points [2]. - pass_cellid: bool, default True. Adds cell array "vtkOriginalCellIds" identifying original cells (note: same cell may appear multiple times) [2]. - nonlinear_subdivision: int, default 1. Sets VTK's SetNonlinearSubdivisionLevel for subdividing nonlinear faces in UnstructuredGrids [1][2][6]. - progress_bar: bool, default False [2]. Examples from 0.43.1 docs use it without 'algorithm', e.g., grid.extract_surface or grid.extract_surface(nonlinear_subdivision=5) [1]. The 'algorithm' parameter allows choosing VTK filters ('geometry'/vtkGeometryFilter, 'dataset_surface'/vtkDataSetSurfaceFilter, None/auto) and is documented in later versions like 0.47.1+ [3][4]. In 0.47+, extract_geometry is deprecated in favor of extract_surface(algorithm='geometry' or None) [7][8]. For 0.43.0 specifically, use the above parameters; upgrade for 'algorithm' support.
Citations:
- 1: https://pyvista-0-43.netlify.app/examples/01-filter/extract-surface
- 2: https://pyvista-0-42.netlify.app/api/core/_autosummary/pyvista.datasetfilters.extract_surface
- 3: https://dev.pyvista.org/api/core/_autosummary/pyvista.dataobjectfilters.extract_surface
- 4: https://docs.pyvista.org/api/core/_autosummary/pyvista.DataObjectFilters.extract_surface.html
- 5: https://docs.pyvista.org/examples/01-filter/extract_surface.html
- 6: add subdivision option to extract_surface pyvista/pyvista#1174
- 7: https://docs.pyvista.org/api/core/_autosummary/pyvista.DataSetFilters.extract_geometry.html
- 8: https://dev.pyvista.org/api/core/_autosummary/pyvista.datasetfilters.extract_geometry
Remove algorithm="dataset_surface" parameter — unsupported in PyVista 0.43.0.
The algorithm parameter was not added to extract_surface() until PyVista 0.47.0. Remove it from lines 75 and any other calls to use the default behavior, or upgrade to PyVista 0.47.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 `@tutorials/tutorial_04_fit_statistical_model_to_patient.py` around lines 69 -
76, The call to pv.DataSet.extract_surface uses the unsupported
algorithm="dataset_surface" parameter (seen where template_data is converted to
template_model); remove the algorithm="dataset_surface" argument and call
extract_surface() with no parameters (or else upgrade PyVista to >=0.47.0 if you
need that option); search for other uses of extract_surface in this file and
remove the algorithm=... parameter there as well to ensure compatibility with
PyVista 0.43.0.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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`:
- Around line 166-168: Update the README entry that currently lists "Jupyter Lab
or Jupyter Notebook" as a requirement and mark it optional (e.g., "Jupyter Lab
or Jupyter Notebook (optional)") or remove it entirely so the doc clearly
reflects the script-first workflow; ensure the surrounding bullets still read
well and that any other mentions of Jupyter in the README are likewise changed
to optional or removed to avoid implying it is required.
🪄 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: f07d8558-d3ea-454c-8fe0-8ce612515046
📒 Files selected for processing (10)
.github/workflows/README.md.github/workflows/ci.yml.github/workflows/release.ymlREADME.mddocs/contributing.rstdocs/faq.rstexperiments/Heart-Create_Statistical_Model/README.mdpyproject.tomltests/GITHUB_WORKFLOWS.mdtests/TESTING_GUIDE.md
✅ Files skipped from review due to trivial changes (5)
- tests/TESTING_GUIDE.md
- .github/workflows/README.md
- tests/GITHUB_WORKFLOWS.md
- docs/contributing.rst
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
save_3d_images: split the single basename argument into (directory,
basename); create the directory if needed. Propagated to all 7 callers
across src/, tests/, experiments/, data/Slicer-Heart-CT, and the
gated-CT workflow.
USDTools: move the module-level load_openusd_as_vtk and
_openusd_display_color helpers onto USDTools as load_usd_as_vtk and
_usd_display_color; update test_tools and test_vtk_to_usd_library
callers.
Tutorial outputs unified under TUTORIALS_DIR / "output" / ... so
tutorials 01/07/08/09 agree with tests/test_tutorials.py.
tutorial_01: drop the stray "test_mode = True" override so
TestTools.running_as_test() actually drives data_dir selection and
registration iteration count.
tutorial_08: validate phase_files, forward_transforms, and
inverse_transforms have equal length and raise ValueError on mismatch
before zipping so dropped frames surface instead of truncating
silently.
experiments/Heart-GatedCT_To_USD/3: move TestTools.running_as_test()
inside the __main__ guard so behavior matches the defensive
Windows-spawn comment.
vtk_to_usd/usd_utils.add_framing_camera: pick the camera offset axis
perpendicular to the stage up axis (+Y for Z-up stages, +Z otherwise)
so Z-up stages frame correctly.
Bump pyvista[all] floor: >=0.43.0 -> >=0.47.0.
Drop legacy Verity{SlicerHeartCT,KCLHeartModel}Data typo shims in
data_download_tools; current code only uses Verify* spellings.
docs/api/cli/index.rst: fix "an main() function" -> "a main() function".
docs/contributing.rst: drop the now-obsolete "If users might copy your
code..." paragraph.
docs/API_MAP.md regenerated.
Comment on lines
238
to
+246
| for array in mesh_data.generic_arrays: | ||
| if triangulation_face_map is not None and array.interpolation == "uniform": | ||
| data = np.asarray(array.data) | ||
| if len(data) == triangulation_face_map.shape[0]: | ||
| # Already triangle-aligned (e.g. derived primvar that was | ||
| # built post-triangulation). Leave it alone. | ||
| pass | ||
| elif len(data) > 0 and triangulation_face_map.max() < len(data): | ||
| expanded_data = data[triangulation_face_map] |
Comment on lines
+71
to
+79
| def add_organ(self, group: str, label_id: int, organ_name: str) -> None: | ||
| """Add one organ label to the named group. | ||
|
|
||
| Creates the group if it does not yet exist. Reassigning the same | ||
| label id within the same group is silently allowed (last write wins); | ||
| the same id appearing in a different group is also allowed but | ||
| :meth:`group_for_id` will then return whichever group was registered | ||
| last. | ||
|
|
Comment on lines
56
to
74
| Args: | ||
| class_name: Subdirectory name for baselines and default results. | ||
| results_dir: Root directory for result artifacts. | ||
| baselines_dir: Root directory for baseline artifacts. | ||
| class_name: Subdirectory name for baselines and, by default, results. | ||
| results_output_dir: Optional exact directory for result artifacts. | ||
| When set, results are read and written there instead of | ||
| ``results_dir / class_name`` while baselines still use | ||
| ``baselines_dir / class_name``. | ||
| results_output_dir: Optional exact directory for written result | ||
| artifacts. Useful when tutorial outputs should live beside | ||
| tutorial files while baselines remain under ``baselines_dir``. | ||
| log_level: Logging level. | ||
| """ | ||
| super().__init__(class_name=class_name, log_level=log_level) | ||
|
|
||
| self._results_dir = ( | ||
| results_output_dir | ||
| if results_output_dir is not None | ||
| else results_dir / class_name | ||
| ) | ||
| self._tests_dir = _REPO_ROOT / "tests" | ||
| if results_output_dir is not None: | ||
| self._results_dir = results_output_dir | ||
| elif results_dir is not None: | ||
| self._results_dir = results_dir | ||
| else: | ||
| self._results_dir = self._tests_dir / "results" / class_name | ||
| self._results_dir.mkdir(parents=True, exist_ok=True) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
==========================================
+ Coverage 27.05% 30.11% +3.06%
==========================================
Files 46 48 +2
Lines 6313 6611 +298
==========================================
+ Hits 1708 1991 +283
- Misses 4605 4620 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/physiomotion4d/usd_tools.py (1)
113-137: 💤 Low valueApply the same time-code fallback to face attributes and the xform cache.
When
time_code is Noneand the points attribute has no default-time value, the code falls back to the first authored point sample but still readsGetFaceVertexCountsAttr().Get(usd_time)/GetFaceVertexIndicesAttr().Get(usd_time)atUsd.TimeCode.Default(), and uses the originalxform_cachefor the transform. For meshes whose topology and/or transform are also authored only at time samples (no default), face_counts becomesNoneand the mesh is silently skipped — even though the points were successfully recovered. Consider promotingsample_timeto drive face attribute lookups and the xform cache when the points fallback fires.♻️ Proposed refactor
- if time_code is None: - usd_time = Usd.TimeCode.Default() - else: - usd_time = Usd.TimeCode(time_code) - xform_cache = UsdGeom.XformCache(usd_time) - - meshes: list[pvtk.PolyData] = [] - for prim in Usd.PrimRange(root_prim): - if not prim.IsA(UsdGeom.Mesh): - continue - - mesh = UsdGeom.Mesh(prim) - points_value = mesh.GetPointsAttr().Get(usd_time) - if points_value is None and time_code is None: - point_samples = mesh.GetPointsAttr().GetTimeSamples() - if point_samples: - sample_time = Usd.TimeCode(point_samples[0]) - points_value = mesh.GetPointsAttr().Get(sample_time) - if points_value is None or len(points_value) == 0: - continue - - face_counts = mesh.GetFaceVertexCountsAttr().Get(usd_time) - face_indices = mesh.GetFaceVertexIndicesAttr().Get(usd_time) + default_time = ( + Usd.TimeCode.Default() if time_code is None else Usd.TimeCode(time_code) + ) + + meshes: list[pvtk.PolyData] = [] + for prim in Usd.PrimRange(root_prim): + if not prim.IsA(UsdGeom.Mesh): + continue + + mesh = UsdGeom.Mesh(prim) + effective_time = default_time + points_value = mesh.GetPointsAttr().Get(effective_time) + if points_value is None and time_code is None: + point_samples = mesh.GetPointsAttr().GetTimeSamples() + if point_samples: + effective_time = Usd.TimeCode(point_samples[0]) + points_value = mesh.GetPointsAttr().Get(effective_time) + if points_value is None or len(points_value) == 0: + continue + + xform_cache = UsdGeom.XformCache(effective_time) + face_counts = mesh.GetFaceVertexCountsAttr().Get(effective_time) + face_indices = mesh.GetFaceVertexIndicesAttr().Get(effective_time)🤖 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/physiomotion4d/usd_tools.py` around lines 113 - 137, The points attribute fallback picks a sample_time but the face attribute lookups and xform cache still use usd_time (Default); update the logic so when time_code is None and you fall back to a sampled time (sample_time) you use that same TimeCode for subsequent reads and transform caching: set usd_time to sample_time (or pass sample_time explicitly) before calling GetFaceVertexCountsAttr().Get(...) and GetFaceVertexIndicesAttr().Get(...), and recreate or reinitialize xform_cache = UsdGeom.XformCache(sample_time) so transforms are read at the same sampled time as points (use the existing symbols time_code, usd_time, sample_time, xform_cache, points_value, face_counts, face_indices).
🤖 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-GatedCT_To_USD/1-register_images.py`:
- Around line 101-104: Before doing the unmasked whole-image registration, clear
any previously set fixed-image mask on the reusable `reg` instance so prior
dynamic/static-step masks don't persist; e.g., call the appropriate method to
clear the fixed mask (such as reg.clear_fixed_mask() or
reg.set_fixed_mask(None)) immediately before reg.set_fixed_image(fixed_image)
and reg.register(moving_image) in the block with reg.set_fixed_image and results
= reg.register(moving_image) (and apply the same clear-before-register change to
the other two occurrences around the reg.register calls at the other noted
locations).
- Around line 31-33: The fixed image is being read from the output_dir which may
be empty; change the source to use data_dir so fixed_image_filename is
constructed from data_dir (e.g. point the "slice_fixed.mha" path to data_dir)
and then call itk.imread on that path; update the variables referenced
(fixed_image_filename, fixed_image, output_dir → data_dir) so itk.imread loads
the input data instead of an output file.
In `@src/physiomotion4d/data_download_tools.py`:
- Around line 38-45: The current logic in DataDownloadTools returns any existing
path (data_file / SLICER_HEART_CT_FILENAME) without verifying integrity and uses
urllib.request.urlretrieve without timeout; change this to download to a
temporary file (e.g., data_file.with_suffix(".tmp") or similar), perform the
network request with an explicit timeout (use urllib.request.urlopen or set
socket timeout), write the response to the temp file, verify basic integrity
(e.g., non-zero size or optional checksum if available), then atomically replace
the target file with os.replace or Path.replace only on success; ensure
SLICER_HEART_CT_URL is the source and the final file name remains
SLICER_HEART_CT_FILENAME.
In `@tests/test_tutorials.py`:
- Around line 67-69: The test calls runpy.run_path(...) which by default does
not execute if __name__ == "__main__" blocks, causing tutorial scripts not to
set tutorial_results; update the runpy invocation in tests/test_tutorials.py
(the runpy.run_path call that uses script_name and expects tutorial_results) to
pass run_name="__main__" so the tutorial's main block executes and
tutorial_results is produced.
---
Nitpick comments:
In `@src/physiomotion4d/usd_tools.py`:
- Around line 113-137: The points attribute fallback picks a sample_time but the
face attribute lookups and xform cache still use usd_time (Default); update the
logic so when time_code is None and you fall back to a sampled time
(sample_time) you use that same TimeCode for subsequent reads and transform
caching: set usd_time to sample_time (or pass sample_time explicitly) before
calling GetFaceVertexCountsAttr().Get(...) and
GetFaceVertexIndicesAttr().Get(...), and recreate or reinitialize xform_cache =
UsdGeom.XformCache(sample_time) so transforms are read at the same sampled time
as points (use the existing symbols time_code, usd_time, sample_time,
xform_cache, points_value, face_counts, face_indices).
🪄 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: 5c9bbfd7-69e4-40b7-844a-560f5b1fa7e4
📒 Files selected for processing (25)
.github/workflows/ci.ymldata/Slicer-Heart-CT/download_and_convert.pydocs/API_MAP.mddocs/api/cli/index.rstdocs/contributing.rstexperiments/Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.pyexperiments/Heart-GatedCT_To_USD/1-register_images.pyexperiments/Heart-GatedCT_To_USD/3-transform_dynamic_and_static_contours.pyexperiments/Heart-VTKSeries_To_USD/0-download_and_convert_4d_to_3d.pypyproject.tomlsrc/physiomotion4d/convert_nrrd_4d_to_3d.pysrc/physiomotion4d/convert_vtk_to_usd.pysrc/physiomotion4d/data_download_tools.pysrc/physiomotion4d/test_tools.pysrc/physiomotion4d/usd_tools.pysrc/physiomotion4d/vtk_to_usd/primvar_derivations.pysrc/physiomotion4d/vtk_to_usd/usd_utils.pysrc/physiomotion4d/workflow_convert_heart_gated_ct_to_usd.pytests/conftest.pytests/test_convert_nrrd_4d_to_3d.pytests/test_tutorials.pytests/test_vtk_to_usd_library.pytutorials/tutorial_01_heart_gated_ct_to_usd.pytutorials/tutorial_07_dirlab_pca_model.pytutorials/tutorial_08_dirlab_pca_time_series.py
✅ Files skipped from review due to trivial changes (2)
- docs/api/cli/index.rst
- tutorials/tutorial_08_dirlab_pca_time_series.py
🚧 Files skipped from review as they are similar to previous changes (13)
- data/Slicer-Heart-CT/download_and_convert.py
- docs/contributing.rst
- src/physiomotion4d/vtk_to_usd/primvar_derivations.py
- .github/workflows/ci.yml
- pyproject.toml
- src/physiomotion4d/vtk_to_usd/usd_utils.py
- tutorials/tutorial_07_dirlab_pca_model.py
- tutorials/tutorial_01_heart_gated_ct_to_usd.py
- experiments/Heart-GatedCT_To_USD/0-download_and_convert_4d_to_3d.py
- src/physiomotion4d/workflow_convert_heart_gated_ct_to_usd.py
- src/physiomotion4d/convert_vtk_to_usd.py
- experiments/Heart-GatedCT_To_USD/3-transform_dynamic_and_static_contours.py
- src/physiomotion4d/test_tools.py
Comment on lines
+31
to
33
| fixed_image_filename = output_dir / "slice_fixed.mha" | ||
| fixed_image = itk.imread(str(fixed_image_filename)) | ||
|
|
There was a problem hiding this comment.
Use the input data directory for the fixed image source.
Line 31 currently reads slice_fixed.mha from output_dir, which is an output folder and may be empty on first run, causing a read failure. Point this to data_dir instead.
Proposed fix
- fixed_image_filename = output_dir / "slice_fixed.mha"
+ fixed_image_filename = data_dir / "slice_fixed.mha"
fixed_image = itk.imread(str(fixed_image_filename))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fixed_image_filename = output_dir / "slice_fixed.mha" | |
| fixed_image = itk.imread(str(fixed_image_filename)) | |
| fixed_image_filename = data_dir / "slice_fixed.mha" | |
| fixed_image = itk.imread(str(fixed_image_filename)) | |
🤖 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 `@experiments/Heart-GatedCT_To_USD/1-register_images.py` around lines 31 - 33,
The fixed image is being read from the output_dir which may be empty; change the
source to use data_dir so fixed_image_filename is constructed from data_dir
(e.g. point the "slice_fixed.mha" path to data_dir) and then call itk.imread on
that path; update the variables referenced (fixed_image_filename, fixed_image,
output_dir → data_dir) so itk.imread loads the input data instead of an output
file.
Comment on lines
+101
to
+104
| # Register the whole image | ||
| reg.set_fixed_image(fixed_image) | ||
| results = reg.register(moving_image) | ||
| inverse_transform = results["inverse_transform"] |
There was a problem hiding this comment.
Reset fixed-mask state before whole-image registration.
Because reg is reused, a mask set in prior dynamic/static steps can persist into later iterations. At Line 101+, clear the fixed mask before the unmasked registration pass.
Proposed fix
# Register the whole image
reg.set_fixed_image(fixed_image)
+ reg.set_fixed_mask(None)
results = reg.register(moving_image)Also applies to: 132-134, 169-171
🤖 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 `@experiments/Heart-GatedCT_To_USD/1-register_images.py` around lines 101 -
104, Before doing the unmasked whole-image registration, clear any previously
set fixed-image mask on the reusable `reg` instance so prior dynamic/static-step
masks don't persist; e.g., call the appropriate method to clear the fixed mask
(such as reg.clear_fixed_mask() or reg.set_fixed_mask(None)) immediately before
reg.set_fixed_image(fixed_image) and reg.register(moving_image) in the block
with reg.set_fixed_image and results = reg.register(moving_image) (and apply the
same clear-before-register change to the other two occurrences around the
reg.register calls at the other noted locations).
Comment on lines
+504
to
+509
| up_axis = UsdGeom.GetStageUpAxis(stage) | ||
| if up_axis == UsdGeom.Tokens.z: | ||
| offset = np.array([0.0, distance, 0.0]) | ||
| else: | ||
| offset = np.array([0.0, 0.0, distance]) | ||
| camera_position = center + offset |
Comment on lines
163
to
+170
| # ConvertVTKToUSD places prims at /World/{basename}/{part_name}. | ||
| # Discover the target prim dynamically so the path stays valid regardless | ||
| # of how many connected components or cell types the VTK file produces. | ||
| mesh_paths = usd_tools.list_mesh_paths_under(stage, parent_path="/World/TPV25Valve") | ||
| if separate_by == "connectivity": | ||
| mesh_paths = usd_tools.list_mesh_paths_under(stage, parent_path="/World/TPV25Valve") | ||
| candidates = [ | ||
| p for p in mesh_paths if p.split("/")[-1].startswith("TPV25Valve_object") | ||
| ] | ||
| vessel_path = candidates[-1] if candidates else "/World/TPV25Valve/Mesh" | ||
| vessel_path = "/World/TPV25Valve/TPV25Valve_object4" | ||
| elif separate_by == "cell_type": | ||
| mesh_paths = usd_tools.list_mesh_paths_under(stage, parent_path="/World/TPV25Valve") | ||
| triangle_paths = [p for p in mesh_paths if p.split("/")[-1].endswith("_Triangle")] | ||
| vessel_path = triangle_paths[0] if triangle_paths else "/World/TPV25Valve/Mesh" | ||
| vessel_path = "/World/TPV25Valve/TPV25Valve_Triangle" |
Comment on lines
165
to
+173
| usd_tools = USDTools() | ||
| # ConvertVTKToUSD places prims at /World/{basename}/{part_name}. | ||
| # Discover the target prim dynamically so the path stays valid regardless | ||
| # of how many connected components or cell types the VTK file produces. | ||
| mesh_paths = usd_tools.list_mesh_paths_under(stage, parent_path="/World/AlterraValve") | ||
| if separate_by == "connectivity": | ||
| mesh_paths = usd_tools.list_mesh_paths_under( | ||
| stage, parent_path="/World/AlterraValve" | ||
| ) | ||
| candidates = [ | ||
| p for p in mesh_paths if p.split("/")[-1].startswith("AlterraValve_object") | ||
| ] | ||
| vessel_path = candidates[-1] if candidates else "/World/AlterraValve/Mesh" | ||
| vessel_path = "/World/AlterraValve/AlterraValve_object3" | ||
| elif separate_by == "cell_type": | ||
| mesh_paths = usd_tools.list_mesh_paths_under( | ||
| stage, parent_path="/World/AlterraValve" | ||
| ) | ||
| triangle_paths = [p for p in mesh_paths if p.split("/")[-1].endswith("_Triangle")] | ||
| vessel_path = triangle_paths[0] if triangle_paths else "/World/AlterraValve/Mesh" | ||
| vessel_path = "/World/AlterraValve/AlterraValve_Triangle" |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/physiomotion4d/test_tools.py (1)
462-462: 💤 Low valueRemove redundant
import os.
osis already imported at module level (line 12), so this local import is unnecessary. Thesysimport on the next line is fine since it's only used in this method.♻️ Proposed fix
- import os import sys🤖 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/physiomotion4d/test_tools.py` at line 462, Remove the redundant local "import os" inside the function in src/physiomotion4d/test_tools.py and rely on the module-level os import; keep the local "import sys" as-is. Locate the stray "import os" statement near the other local imports in the function (adjacent to the local "import sys") and delete it so there is a single module-level os import.
🤖 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 `@src/physiomotion4d/data_download_tools.py`:
- Around line 49-67: The tmp_file currently used in DataDownloadTools.download
(tmp_file = data_file.with_suffix(...)) is a fixed path and can be clobbered by
concurrent callers; change the implementation to create a unique temporary file
in the same directory (e.g., use
tempfile.NamedTemporaryFile(dir=data_file.parent, delete=False) or generate a
UUID-based filename) so each call gets its own temp path, write the download
into that unique temp file, verify size and then atomically replace data_file
(tmp.replace(data_file) or os.replace), and ensure the existing cleanup logic
(unlink on exception) removes the unique temp file instead of the constant name.
In `@src/physiomotion4d/test_tools.py`:
- Around line 474-503: When pv.start_xvfb() is called you must ensure
pv.stop_xvfb() runs even if USDTools().load_usd_as_vtk(...) raises; change the
flow so starting Xvfb and setting xvfb_started is immediately followed by a
try/finally that calls pv.stop_xvfb() in the finally, then perform
USDTools.load_usd_as_vtk, plotter creation and screenshot inside that try block;
reference pv.start_xvfb / pv.stop_xvfb and USDTools().load_usd_as_vtk() and
ensure plotter.close() remains in its own finally or the same inner try/finally
so both plotter.close() and pv.stop_xvfb() are always executed.
---
Nitpick comments:
In `@src/physiomotion4d/test_tools.py`:
- Line 462: Remove the redundant local "import os" inside the function in
src/physiomotion4d/test_tools.py and rely on the module-level os import; keep
the local "import sys" as-is. Locate the stray "import os" statement near the
other local imports in the function (adjacent to the local "import sys") and
delete it so there is a single module-level os import.
🪄 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: 5a01f19a-8cb9-45cd-ae19-41f827249740
📒 Files selected for processing (9)
docs/API_MAP.mdexperiments/Heart-GatedCT_To_USD/1-register_images.pysrc/physiomotion4d/anatomy_taxonomy.pysrc/physiomotion4d/data_download_tools.pysrc/physiomotion4d/test_tools.pysrc/physiomotion4d/vtk_to_usd/usd_mesh_converter.pysrc/physiomotion4d/vtk_to_usd/usd_utils.pytests/test_register_time_series_images.pytests/test_tutorials.py
🚧 Files skipped from review as they are similar to previous changes (4)
- src/physiomotion4d/anatomy_taxonomy.py
- experiments/Heart-GatedCT_To_USD/1-register_images.py
- src/physiomotion4d/vtk_to_usd/usd_utils.py
- tests/test_tutorials.py
Comment on lines
+28
to
36
| Each tutorial is a standalone percent-cell Python script (`# %%`) that can be | ||
| run cell-by-cell in VS Code or Cursor without Jupyter packages. Paths are | ||
| defined near the top of each script. By default, data is read from the | ||
| repository `data/` directory and outputs are written under `output/`. | ||
|
|
||
| ```bash | ||
| python tutorials/tutorial_01_heart_gated_ct_to_usd.py \ | ||
| --data-dir ./data --output-dir ./output | ||
|
|
||
| python tutorials/tutorial_02_ct_to_vtk.py \ | ||
| --data-dir ./data --output-dir ./output | ||
| # In VS Code/Cursor, open the tutorial and run the final cell: | ||
| tutorial_results = run_tutorial() | ||
| ``` |
Comment on lines
+1
to
+6
| """Tutorial 7: Build and fit a PCA lung-lobe model from DirLab 4D CT cases. | ||
|
|
||
| This tutorial uses one respiratory phase from each available DirLab case. It | ||
| segments the five lung lobes, builds a surface PCA model from all but two | ||
| cases, then fits that model to every available case. | ||
| """ |
Comment on lines
+1
to
+15
| """ | ||
| Tutorial 8: Propagate DirLab PCA lung-lobe fits through each 4D CT time series. | ||
|
|
||
| This tutorial uses the per-case PCA-fitted reference meshes created by Tutorial | ||
| 7. For each DirLab case, it registers every respiratory phase to the reference | ||
| phase used by Tutorial 7, saves the image transforms, and applies the | ||
| reference-to-phase transform to the fitted mesh so each time point has a | ||
| PCA-derived lung-lobe surface. | ||
|
|
||
| Data Required | ||
| ------------- | ||
| See data/README.md for DirLab-4DCT download instructions. Run Tutorial 7 first | ||
| so ``output/tutorial_07_dirlab_pca_model/fits`` contains the per-case fitted | ||
| reference-stage meshes. | ||
| """ |
Comment on lines
+139
to
+150
| world_matrix = xform_cache.GetLocalToWorldTransform(prim) | ||
| points = np.asarray( | ||
| [ | ||
| tuple( | ||
| world_matrix.Transform( | ||
| Gf.Vec3d(float(point[0]), float(point[1]), float(point[2])) | ||
| ) | ||
| ) | ||
| for point in points_value | ||
| ], | ||
| dtype=np.float32, | ||
| ) |
Comment on lines
162
to
+170
| usd_tools = USDTools() | ||
| # ConvertVTKToUSD places prims at /World/{basename}/{part_name}. | ||
| # Discover the target prim dynamically so the path stays valid regardless | ||
| # of how many connected components or cell types the VTK file produces. | ||
| mesh_paths = usd_tools.list_mesh_paths_under(stage, parent_path="/World/TPV25Valve") | ||
| if separate_by == "connectivity": | ||
| mesh_paths = usd_tools.list_mesh_paths_under(stage, parent_path="/World/TPV25Valve") | ||
| candidates = [ | ||
| p for p in mesh_paths if p.split("/")[-1].startswith("TPV25Valve_object") | ||
| ] | ||
| vessel_path = candidates[-1] if candidates else "/World/TPV25Valve/Mesh" | ||
| vessel_path = "/World/TPV25Valve/TPV25Valve_object4" | ||
| elif separate_by == "cell_type": | ||
| mesh_paths = usd_tools.list_mesh_paths_under(stage, parent_path="/World/TPV25Valve") | ||
| triangle_paths = [p for p in mesh_paths if p.split("/")[-1].endswith("_Triangle")] | ||
| vessel_path = triangle_paths[0] if triangle_paths else "/World/TPV25Valve/Mesh" | ||
| vessel_path = "/World/TPV25Valve/TPV25Valve_Triangle" |
Comment on lines
165
to
175
| usd_tools = USDTools() | ||
| # ConvertVTKToUSD places prims at /World/{basename}/{part_name}. | ||
| # Discover the target prim dynamically so the path stays valid regardless | ||
| # of how many connected components or cell types the VTK file produces. | ||
| mesh_paths = usd_tools.list_mesh_paths_under(stage, parent_path="/World/AlterraValve") | ||
| if separate_by == "connectivity": | ||
| mesh_paths = usd_tools.list_mesh_paths_under( | ||
| stage, parent_path="/World/AlterraValve" | ||
| ) | ||
| candidates = [ | ||
| p for p in mesh_paths if p.split("/")[-1].startswith("AlterraValve_object") | ||
| ] | ||
| vessel_path = candidates[-1] if candidates else "/World/AlterraValve/Mesh" | ||
| vessel_path = "/World/AlterraValve/AlterraValve_object3" | ||
| elif separate_by == "cell_type": | ||
| mesh_paths = usd_tools.list_mesh_paths_under( | ||
| stage, parent_path="/World/AlterraValve" | ||
| ) | ||
| triangle_paths = [p for p in mesh_paths if p.split("/")[-1].endswith("_Triangle")] | ||
| vessel_path = triangle_paths[0] if triangle_paths else "/World/AlterraValve/Mesh" | ||
| vessel_path = "/World/AlterraValve/AlterraValve_Triangle" | ||
| else: | ||
| vessel_path = "/World/AlterraValve/Mesh" |
Comment on lines
144
to
147
| ## Test-Mode Flag: PHYSIOMOTION_RUNNING_AS_TEST | ||
|
|
||
| When experiment tests run (with `--run-experiments`), the test runner also sets **`PHYSIOMOTION_RUNNING_AS_TEST=1`** so that notebooks can use reduced parameters (e.g. fewer iterations, fewer files) and finish faster. Notebooks should read this variable and choose quick vs full parameters accordingly. See [EXPERIMENT_TESTS_GUIDE.md](EXPERIMENT_TESTS_GUIDE.md#running-as-test-physiomotion_running_as_test) for the recommended check and the `physiomotion4d.notebook_utils.running_as_test()` helper. | ||
| When experiment tests run (with `--run-experiments`), the test runner also sets **`PHYSIOMOTION_RUNNING_AS_TEST=1`** so that notebooks can use reduced parameters (e.g. fewer iterations, fewer files) and finish faster. Notebooks should read this variable and choose quick vs full parameters accordingly. See [EXPERIMENT_TESTS_GUIDE.md](EXPERIMENT_TESTS_GUIDE.md#running-as-test-physiomotion_running_as_test) for the recommended check and the `physiomotion4d.test_tools.TestTools.running_as_test()` helper. | ||
|
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/EXPERIMENT_TESTS_GUIDE.md (1)
211-217: 💤 Low valueConsider adding a language specifier to the fenced code block.
The code block showing script execution output doesn't specify a language. Adding
textorconsolewould make the formatting intent clearer and satisfy the markdownlint rule.📝 Optional improvement
-``` +```text ================================================================================ Executing script: 1-register_images.py Path: experiments/Heart-GatedCT_To_USD/1-register_images.py🤖 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 `@tests/EXPERIMENT_TESTS_GUIDE.md` around lines 211 - 217, The fenced code block that contains the script execution output (the triple-backtick block starting with "================================================================================" and the "Executing script: 1-register_images.py" lines) lacks a language specifier; update that opening fence from ``` to ```text (or ```console) so the block is explicitly marked as plain text and satisfies markdownlint; make the change in the EXPERIMENT_TESTS_GUIDE.md fenced block that contains the example output.
🤖 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 `@data/README.md`:
- Around line 282-284: Insert a single blank line between the "### Summary of
Download Methods" heading and the following table (the pipe-delimited table
starting with "| Dataset | Auto-Download | ...") so the heading and table are
separated; this fixes markdownlint MD058 by ensuring there is an empty line
before the table.
In `@README.md`:
- Line 812: There are duplicate "## Documentation" headings causing ambiguous
anchors (MD024); locate the second occurrence of the heading (the literal line
"## Documentation") and rename it to a unique title (e.g., "## Additional
Documentation" or "## Documentation (Supplementary)"), then update any internal
links/TOC entries that pointed to the old "documentation" anchor so they
reference the new heading text.
In `@statistics.md`:
- Line 36: The documentation row in statistics.md showing " ~7,800 md" is
incorrect; update the Markdown count in the "Documentation (`docs/` + repo
READMEs)" row to the accurate value (~6,701 or rounded to ~6,700) and adjust the
combined lines and percentage accordingly (update the combined total to ~13,000
lines and the percent to ~31% of 42,000), or alternatively recompute and replace
both the markdown count and the overall combined/percentage values so the row is
consistent and accurate.
In `@tests/EXPERIMENT_TESTS_GUIDE.md`:
- Around line 168-172: The Option 1 example uses os.environ.get but omits the
required import; add "import os" before the inline statement so the example is
runnable—update the snippet containing running_as_test to include the import
line above it.
In `@tests/PARALLEL_EXECUTION_GUIDE.md`:
- Around line 158-160: The fenced code block containing the line "0-download.py
-> 1-process.py -> 2-analyze.py -> 3-visualize.py" is missing a language tag
(triggering markdownlint MD040); update the block start from ``` to ```text so
it becomes a fenced text block (e.g., change the opening fence to ```text) to
satisfy the linter while keeping the content identical.
---
Nitpick comments:
In `@tests/EXPERIMENT_TESTS_GUIDE.md`:
- Around line 211-217: The fenced code block that contains the script execution
output (the triple-backtick block starting with
"================================================================================"
and the "Executing script: 1-register_images.py" lines) lacks a language
specifier; update that opening fence from ``` to ```text (or ```console) so the
block is explicitly marked as plain text and satisfies markdownlint; make the
change in the EXPERIMENT_TESTS_GUIDE.md fenced block that contains the example
output.
🪄 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: 61aff6a2-3b95-45f5-9bd3-4c1ff07d73f9
📒 Files selected for processing (31)
.github/workflows/README.mdCLAUDE.mdREADME.mddata/KCL-Heart-Model/README.mddata/README.mddocs/API_MAP.mddocs/README.mddocs/cli_scripts/create_statistical_model.rstdocs/contributing.rstdocs/examples.rstdocs/quickstart.rstexperiments/Convert_VTK_To_USD/convert_chop_alterra_valve_to_usd.pyexperiments/Convert_VTK_To_USD/convert_chop_tpv25_valve_to_usd.pyexperiments/Heart-Create_Statistical_Model/README.mdexperiments/Heart-Simpleware_Segmentation/README.mdexperiments/README.mdsrc/physiomotion4d/data_download_tools.pysrc/physiomotion4d/segment_anatomy_base.pysrc/physiomotion4d/segment_chest_total_segmentator.pysrc/physiomotion4d/simpleware_medical/README.mdsrc/physiomotion4d/test_tools.pysrc/physiomotion4d/usd_tools.pystatistics.mdtests/EXPERIMENT_FLAG_USAGE.mdtests/EXPERIMENT_TESTS_GUIDE.mdtests/EXPERIMENT_TESTS_SUMMARY.mdtests/PARALLEL_EXECUTION_GUIDE.mdtests/README.mdtests/TESTING_GUIDE.mdtests/test_convert_nrrd_4d_to_3d.pytutorials/README.md
✅ Files skipped from review due to trivial changes (10)
- src/physiomotion4d/simpleware_medical/README.md
- CLAUDE.md
- data/KCL-Heart-Model/README.md
- docs/cli_scripts/create_statistical_model.rst
- docs/examples.rst
- docs/README.md
- tests/EXPERIMENT_FLAG_USAGE.md
- experiments/Heart-Create_Statistical_Model/README.md
- experiments/Heart-Simpleware_Segmentation/README.md
- docs/quickstart.rst
🚧 Files skipped from review as they are similar to previous changes (10)
- tests/TESTING_GUIDE.md
- experiments/README.md
- tests/test_convert_nrrd_4d_to_3d.py
- src/physiomotion4d/segment_chest_total_segmentator.py
- experiments/Convert_VTK_To_USD/convert_chop_tpv25_valve_to_usd.py
- src/physiomotion4d/test_tools.py
- src/physiomotion4d/segment_anatomy_base.py
- src/physiomotion4d/data_download_tools.py
- experiments/Convert_VTK_To_USD/convert_chop_alterra_valve_to_usd.py
- src/physiomotion4d/usd_tools.py
Comment on lines
325
to
333
| try: | ||
| urllib.request.urlretrieve(input_image_url, str(input_image_filename)) | ||
| print(f"Downloaded to {input_image_filename}") | ||
| except urllib.error.URLError as e: | ||
| input_image_filename = DataDownloadTools.DownloadSlicerHeartCTData(data_dir) | ||
| print(f"\nSlicer-Heart-CT data ready: {input_image_filename}") | ||
| except OSError as e: | ||
| msg = ( | ||
| f"Could not download test data: {e}. " | ||
| f"Please manually place TruncalValve_4DCT.seq.nrrd in {data_dir}" | ||
| "Please manually place " | ||
| f"{DataDownloadTools.SLICER_HEART_CT_FILENAME} in {data_dir}" | ||
| ) |
Comment on lines
+113
to
+139
| if time_code is None: | ||
| usd_time = Usd.TimeCode.Default() | ||
| else: | ||
| usd_time = Usd.TimeCode(time_code) | ||
| xform_cache = UsdGeom.XformCache(usd_time) | ||
|
|
||
| meshes: list[pvtk.PolyData] = [] | ||
| for prim in Usd.PrimRange(root_prim): | ||
| if not prim.IsA(UsdGeom.Mesh): | ||
| continue | ||
|
|
||
| mesh = UsdGeom.Mesh(prim) | ||
| points_value = mesh.GetPointsAttr().Get(usd_time) | ||
| if points_value is None and time_code is None: | ||
| point_samples = mesh.GetPointsAttr().GetTimeSamples() | ||
| if point_samples: | ||
| sample_time = Usd.TimeCode(point_samples[0]) | ||
| points_value = mesh.GetPointsAttr().Get(sample_time) | ||
| if points_value is None or len(points_value) == 0: | ||
| continue | ||
|
|
||
| face_counts = mesh.GetFaceVertexCountsAttr().Get(usd_time) | ||
| face_indices = mesh.GetFaceVertexIndicesAttr().Get(usd_time) | ||
| if face_counts is None or face_indices is None: | ||
| continue | ||
|
|
||
| world_matrix = xform_cache.GetLocalToWorldTransform(prim) |
Comment on lines
+369
to
+385
| def triangulate_face( | ||
| face_counts: NDArray, face_indices: NDArray | ||
| ) -> tuple[NDArray, NDArray, NDArray]: | ||
| """Triangulate polygonal faces using fan triangulation. | ||
|
|
||
| Args: | ||
| face_counts: Array of vertex counts per face | ||
| face_indices: Array of vertex indices | ||
| face_counts: Array of vertex counts per source face (length F). | ||
| face_indices: Flat array of vertex indices. | ||
|
|
||
| Returns: | ||
| tuple: (triangulated_counts, triangulated_indices) | ||
| ``(tri_counts, tri_indices, source_face_index_per_triangle)``: | ||
| - ``tri_counts``: int32 array, all entries equal to 3. | ||
| - ``tri_indices``: int32 flat array of triangle vertex indices. | ||
| - ``source_face_index_per_triangle``: int32 array mapping each output | ||
| triangle back to its source face in ``face_counts``. Length matches | ||
| ``tri_counts``. Use this to expand uniform (per-face) primvar data | ||
| to match the triangulated face count: ``new_data = old_data[mapping]``. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the per-organ flat dicts on SegmentAnatomyBase and USDAnatomyTools with a shared AnatomyTaxonomy data type so new segmenters can register custom groups via taxonomy.add_organ without subclassing either class. Adds anatomy-type grouping to ConvertVTKToUSD's labeled output, ships tutorials 07-09 (DirLab PCA + PhysicsNeMo), and fixes several USD- pipeline issues that prevented clean rendering in Omniverse Kit.
Anatomy / segmentation
USD generation
Tutorials / experiments
Tests, docs, packaging
Other
Summary by CodeRabbit
New Features
Documentation
Chores