ENH: Provides volumetric meshing.#78
Merged
Merged
Conversation
WalkthroughThis PR adds netgen-based tetrahedral mesh generation to the VTK workflow, exposes new surface/mesh reduction controls in the CLI and tutorial, updates downstream callers to ChangesVTK netgen mesh conversion
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant convert_image_to_vtk CLI
participant WorkflowConvertImageToVTK
participant ContourTools
User->>convert_image_to_vtk CLI: run with reduction options
convert_image_to_vtk CLI->>WorkflowConvertImageToVTK: process(input_image, surface_target_reduction, mesh_target_reduction)
loop per anatomy group
WorkflowConvertImageToVTK->>ContourTools: extract_mesh(base_surface, mesh_target_reduction)
WorkflowConvertImageToVTK->>ContourTools: save_surfaces / save_meshes or combined variants
end
WorkflowConvertImageToVTK-->>convert_image_to_vtk CLI: surfaces and meshes
convert_image_to_vtk CLI-->>User: written VTP and VTU files
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/physiomotion4d/segment_chest_total_segmentator_with_contrast.py (1)
55-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBase class hook still typed with lowercase
itk.image, creating an inconsistent override signature.
postprocess_after_labelmapnow usesitk.Imagehere, but the upstream hook it overrides —SegmentAnatomyBase.postprocess_after_labelmapinsegment_anatomy_base.py:330-349— is still typed with the lowercaseitk.image. Web search confirms ITK's Python wrapping only exposes the capitalizeditk.Imagetemplate class; there is no documenteditk.imagesymbol, so the base class's annotation was likely relying on an undefined/incorrect name all along. Now that this file fixes the type toitk.Image, the override diverges from its base declaration, which under mypy strict mode (disallow_untyped_defs = true, per coding guidelines) can surface as an incompatible-override warning once the base signature is checked against an actual symbol.Since the base class file isn't part of this cohort, consider tracking a follow-up to update
SegmentAnatomyBase.postprocess_after_labelmap's hints toitk.Imagefor consistency.Also applies to: 90-91, 102-102
🤖 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/segment_chest_total_segmentator_with_contrast.py` around lines 55 - 80, The override signature for postprocess_after_labelmap is now using itk.Image, but the base hook SegmentAnatomyBase.postprocess_after_labelmap still needs the same annotation to keep the contract consistent. Update the base method’s type hints from the incorrect lowercase itk.image to itk.Image so the override matches cleanly and mypy does not report an incompatible override; use the postprocess_after_labelmap symbol in SegmentAnatomyBase to locate the declaration.src/physiomotion4d/cli/convert_image_to_vtk.py (1)
112-131: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd range validation for the new decimation fractions.
Both
--surface-target-reductionand--mesh-target-reductionare documented as fractions in[0, 1), buttype=floataccepts any value (negative, ≥1, NaN). Invalid values will flow unchecked intodecimate_pro/netgen deep inrun_workflow, producing a confusing failure instead of a clear CLI error.🛡️ Proposed fix to validate range at parse time
+def _fraction_below_one(value: str) -> float: + fval = float(value) + if not (0.0 <= fval < 1.0): + raise argparse.ArgumentTypeError(f"must be in [0, 1), got {fval}") + return fval + parser.add_argument( "--surface-target-reduction", - type=float, + type=_fraction_below_one, default=0.0, help=( "Fraction in [0, 1) of surface triangles to remove via " "decimate_pro (default: 0.0, no decimation)." ), ) parser.add_argument( "--mesh-target-reduction", - type=float, + type=_fraction_below_one, default=0.0, help=( "Fraction in [0, 1) of triangles to remove from the surface " "(via decimate_pro) before it is meshed into a tetrahedral " "volume mesh by netgen; a coarser input surface yields a " "coarser volume mesh (default: 0.0, no decimation)." ), )🤖 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/cli/convert_image_to_vtk.py` around lines 112 - 131, Add parse-time range validation for the new CLI options so invalid decimation fractions are rejected early. Update the argument handling in convert_image_to_vtk.py for `--surface-target-reduction` and `--mesh-target-reduction` to enforce values in `[0, 1)` instead of relying on plain `type=float`. Prefer a small reusable validator/helper used by `parser.add_argument` so `run_workflow` never receives negative, >=1, or NaN values, and the CLI reports a clear error.src/physiomotion4d/workflow_convert_image_to_vtk.py (1)
245-250: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider guarding
GenerateVolumeMesh()so one bad group doesn't abort the whole workflow.
GenerateVolumeMesh()is called per anatomy group insiderun_workflow's loop after an expensive segmentation step. netgen can raise (or, per your own docstring, misbehave) on surfaces that aren't clean/watertight; an uncaught exception here loses every already-computed group. Since the function already degrades gracefully toNonewhen no 3D elements are produced, treating a meshing failure the same way would be more resilient.♻️ Suggested resilience handling
- ngmesh.GenerateVolumeMesh() - - elements = ngmesh.Elements3D() + try: + ngmesh.GenerateVolumeMesh() + except Exception as exc: # netgen may fail on non-watertight input + self.log_warning("netgen volume meshing failed for this surface: %s", exc) + return None + + elements = ngmesh.Elements3D()🤖 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_image_to_vtk.py` around lines 245 - 250, Guard the meshing step in `run_workflow`/`workflow_convert_image_to_vtk.py` around `ngmesh.GenerateVolumeMesh()` so a failure for one anatomy group does not abort the entire loop. Catch the exception from `GenerateVolumeMesh()` in the same place where `Elements3D()` is checked, log a warning for that group, and return `None` just like the existing “no volume mesh” path. This keeps the per-group workflow resilient while preserving already-computed 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.
Nitpick comments:
In `@src/physiomotion4d/cli/convert_image_to_vtk.py`:
- Around line 112-131: Add parse-time range validation for the new CLI options
so invalid decimation fractions are rejected early. Update the argument handling
in convert_image_to_vtk.py for `--surface-target-reduction` and
`--mesh-target-reduction` to enforce values in `[0, 1)` instead of relying on
plain `type=float`. Prefer a small reusable validator/helper used by
`parser.add_argument` so `run_workflow` never receives negative, >=1, or NaN
values, and the CLI reports a clear error.
In `@src/physiomotion4d/segment_chest_total_segmentator_with_contrast.py`:
- Around line 55-80: The override signature for postprocess_after_labelmap is
now using itk.Image, but the base hook
SegmentAnatomyBase.postprocess_after_labelmap still needs the same annotation to
keep the contract consistent. Update the base method’s type hints from the
incorrect lowercase itk.image to itk.Image so the override matches cleanly and
mypy does not report an incompatible override; use the
postprocess_after_labelmap symbol in SegmentAnatomyBase to locate the
declaration.
In `@src/physiomotion4d/workflow_convert_image_to_vtk.py`:
- Around line 245-250: Guard the meshing step in
`run_workflow`/`workflow_convert_image_to_vtk.py` around
`ngmesh.GenerateVolumeMesh()` so a failure for one anatomy group does not abort
the entire loop. Catch the exception from `GenerateVolumeMesh()` in the same
place where `Elements3D()` is checked, log a warning for that group, and return
`None` just like the existing “no volume mesh” path. This keeps the per-group
workflow resilient while preserving already-computed results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5fb6bc23-4c71-4cc3-a15f-7e67f6568a8a
📒 Files selected for processing (7)
docs/api/workflows.rstdocs/troubleshooting.rstpyproject.tomlsrc/physiomotion4d/cli/convert_image_to_vtk.pysrc/physiomotion4d/segment_chest_total_segmentator_with_contrast.pysrc/physiomotion4d/workflow_convert_image_to_vtk.pytutorials/tutorial_02_ct_to_vtk.py
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #78 +/- ##
==========================================
- Coverage 36.06% 36.01% -0.05%
==========================================
Files 58 58
Lines 7271 7292 +21
==========================================
+ Hits 2622 2626 +4
- Misses 4649 4666 +17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR enhances the image-to-VTK pipeline by switching volumetric meshing from voxel/hexahedral thresholding to netgen-generated tetrahedral volume meshes and exposing surface/mesh decimation controls through the workflow API, CLI, and tutorial/docs.
Changes:
- Replace VTU generation with netgen-based tetrahedral volume meshing derived from extracted surfaces.
- Add
surface_target_reductionandmesh_target_reductionparameters toWorkflowConvertImageToVTK.run_workflow()and surface them in the CLI and tutorial. - Update documentation and dependencies to include netgen support.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tutorials/tutorial_02_ct_to_vtk.py | Updates tutorial wording and demonstrates new decimation parameters when running the workflow. |
| src/physiomotion4d/workflow_convert_image_to_vtk.py | Implements netgen tetrahedral meshing, adds reduction parameters, and updates workflow docstrings. |
| src/physiomotion4d/segment_chest_total_segmentator_with_contrast.py | Tightens type hints/return annotations for contrast segmenter methods. |
| src/physiomotion4d/cli/convert_image_to_vtk.py | Updates CLI text to “tetrahedral volume mesh” and adds flags for surface/mesh reduction. |
| pyproject.toml | Adds netgen-mesher dependency and configures mypy to ignore netgen modules. |
| docs/troubleshooting.rst | Adjusts example imports in troubleshooting docs. |
| docs/api/workflows.rst | Updates USD workflow example argument name (usd_project_name). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
227
to
231
| meshing_surface = surface.triangulate().clean() | ||
| if mesh_target_reduction > 0.0: | ||
| meshing_surface = meshing_surface.decimate_pro( | ||
| mesh_target_reduction, preserve_topology=True | ||
| ) |
Comment on lines
+357
to
+361
| export_surface = base_surface | ||
| if surface_target_reduction > 0.0: | ||
| export_surface = export_surface.decimate_pro( | ||
| surface_target_reduction, preserve_topology=True | ||
| ) |
Comment on lines
+233
to
+235
| import netgen.meshing as ngm # noqa: PLC0415 | ||
|
|
||
| triangles = meshing_surface.faces.reshape(-1, 4)[:, 1:4] |
Comment on lines
187
to
192
| result = workflow.run_workflow( | ||
| input_image=input_image, | ||
| anatomy_groups=args.anatomy_groups, | ||
| surface_target_reduction=args.surface_target_reduction, | ||
| mesh_target_reduction=args.mesh_target_reduction, | ||
| ) |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/physiomotion4d/cli/convert_image_to_vtk.py (1)
112-131: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate the reduction flags in the CLI parser. Both
--surface-target-reductionand--mesh-target-reductionare documented as fractions in[0, 1), but the CLI accepts any float and passes it through unchanged. Add parse-time range checks so1.0or negative values fail with a clear argument error instead of surfacing later fromdecimate_pro.🤖 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/cli/convert_image_to_vtk.py` around lines 112 - 131, Validate the CLI reduction flags in the parser so `--surface-target-reduction` and `--mesh-target-reduction` reject values outside [0, 1) at parse time instead of passing them through to `decimate_pro`. Update the `argparse` setup in `convert_image_to_vtk.py` to use a range-checking type or validation helper for both arguments, and make sure invalid values like negatives or 1.0 raise a clear `ArgumentTypeError`/parser error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/physiomotion4d/cli/convert_image_to_vtk.py`:
- Around line 112-131: Validate the CLI reduction flags in the parser so
`--surface-target-reduction` and `--mesh-target-reduction` reject values outside
[0, 1) at parse time instead of passing them through to `decimate_pro`. Update
the `argparse` setup in `convert_image_to_vtk.py` to use a range-checking type
or validation helper for both arguments, and make sure invalid values like
negatives or 1.0 raise a clear `ArgumentTypeError`/parser error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7636b002-cf1a-4471-9e34-3006093bbafa
📒 Files selected for processing (7)
docs/api/workflows.rstsrc/physiomotion4d/cli/convert_image_to_vtk.pysrc/physiomotion4d/contour_tools.pysrc/physiomotion4d/workflow_convert_image_to_vtk.pysrc/physiomotion4d/workflow_fit_statistical_model_to_patient.pytests/test_workflow_fit_statistical_model_to_patient.pytutorials/tutorial_02_ct_to_vtk.py
✅ Files skipped from review due to trivial changes (1)
- docs/api/workflows.rst
🚧 Files skipped from review as they are similar to previous changes (1)
- tutorials/tutorial_02_ct_to_vtk.py
Comment on lines
25
to
+27
| # Combined single-file output (default) | ||
| WorkflowConvertImageToVTK.save_combined_surface(result['surfaces'], './out', prefix='patient') | ||
| WorkflowConvertImageToVTK.save_combined_mesh(result['meshes'], './out', prefix='patient') | ||
| ContourTools.save_combined_surface(result['surfaces'], './out', prefix='patient') | ||
| ContourTools.save_combined_mesh(result['meshes'], './out', prefix='patient') |
Comment on lines
69
to
73
| Each :class:`pyvista.PolyData` surface and :class:`pyvista.UnstructuredGrid` mesh | ||
| returned by :meth:`run_workflow` carries: | ||
| returned by :meth:`process` carries: | ||
|
|
||
| - ``field_data['AnatomyGroup']`` — anatomy group name, e.g. ``'heart'``. | ||
| - ``field_data['SegmentationLabelNames']`` — individual structure names within the |
Comment on lines
+85
to
+86
| :meth:`ContourTools.save_combined_mesh` — or the CLI | ||
| ``physiomotion4d-convert-image-to-vtk`` — to write results to disk. |
Comment on lines
+230
to
+234
| mesh_target_reduction: Fraction in ``[0, 1)`` of the surface's | ||
| triangles to remove (via the same ``decimate_pro`` call) | ||
| before it is handed to netgen for tetrahedral meshing. A | ||
| coarser input surface yields a coarser volume mesh — netgen | ||
| itself has no post-hoc decimation. ``0.0`` (default) skips |
Comment on lines
+300
to
+304
| export_surface = base_surface | ||
| if surface_target_reduction > 0.0: | ||
| export_surface = export_surface.decimate_pro( | ||
| surface_target_reduction, preserve_topology=True | ||
| ) |
Comment on lines
+583
to
+584
| meshes: Mapping of name → mesh (e.g. the ``'meshes'`` value from | ||
| :meth:`WorkflowConvertImageToVTK.process`). |
| ``{prefix}_{name}.vtu`` (or ``{name}.vtu`` when *prefix* is empty). | ||
|
|
||
| Returns: | ||
| Mapping of name → absolute path of the saved file. |
| in the merged file. | ||
|
|
||
| Args: | ||
| surfaces: Mapping of name → surface. |
| merged file. | ||
|
|
||
| Args: | ||
| meshes: Mapping of name → volume mesh. |
Comment on lines
38
to
42
| Output files — combined mode (default) | ||
| --------------------------------------- | ||
| {prefix}_surfaces.vtp all surfaces merged into one file | ||
| {prefix}_meshes.vtu all voxel meshes merged into one file | ||
| {prefix}_meshes.vtu all tetrahedral volume meshes merged into one file | ||
|
|
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.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
process(...)flow and new parameters.