Support multi-point SeedpointVolume across volume/surface translators and GAI seed points#1982
Support multi-point SeedpointVolume across volume/surface translators and GAI seed points#1982shreyas-flex wants to merge 3 commits intomainfrom
Conversation
shreyas-flex
requested review from
NasserFlexCompute,
benflexcompute and
piotrkluba
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3413d3c. Configure here.
| @contextual_model_validator(mode="after") | ||
| def _check_seedpoint_volume_usage(self, param_info: ParamsValidationInfo): | ||
| """Validate SeedpointVolume usage in modular meshing schema.""" | ||
| _validate_seedpoint_volume_usage(_collect_all_seedpoint_volumes(self.zones), param_info) |
There was a problem hiding this comment.
Validation regression: unconditional check replaced with context-dependent one
Medium Severity
The old _check_snappy_zones else branch unconditionally rejected SeedpointVolume with non-snappy meshers (plain @pd.model_validator). The replacement _check_seedpoint_volume_usage is a @contextual_model_validator that only runs when a ParamsValidationInfo context is provided. When a ModularMeshingWorkflow is constructed without a validation context (e.g., in notebooks or scripts), SeedpointVolume entries paired with a non-snappy, non-beta mesher will silently pass validation instead of raising an error.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3413d3c. Configure here.
There was a problem hiding this comment.
Pull request overview
Adds multi-point SeedpointVolume support end-to-end (schema, translators, and tests), while keeping snappyHexMeshing constrained to a single locationInMesh point per zone and emitting GAI meshing.defaults.seed_points.
Changes:
- Extend
SeedpointVolume.point_in_meshto accept one-or-many points (normalized to a list). - Update volume/surface translators to emit
seedPoints(volume) anddefaults.seed_points(GAI), and enforce single-point snappylocationInMesh. - Add/adjust validation + tests to cover the new multi-point behavior and mesher compatibility rules.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| flow360/component/simulation/primitives.py | Allows SeedpointVolume to accept and normalize multiple seed points. |
| flow360/component/simulation/meshing_param/params.py | Validates SeedpointVolume usage is restricted to snappyHexMeshing or beta mesher. |
| flow360/component/simulation/translator/volume_meshing_translator.py | Emits seedPoints: [[x,y,z], ...] for seedpoint zones in volume meshing JSON. |
| flow360/component/simulation/translator/surface_meshing_translator.py | Enforces single-point snappy locationInMesh and emits GAI defaults.seed_points. |
| tests/simulation/params/meshing_validation/test_meshing_param_validation.py | Adds validation coverage for allowed meshers when using SeedpointVolume. |
| tests/simulation/translator/test_volume_meshing_translator.py | Updates expected JSON and adds multi-point seedPoints translation test. |
| tests/simulation/translator/test_surface_meshing_translator.py | Adds snappy rejection test for multi-point seed volumes and GAI seed point emission test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "remove_hidden_geometry": None, | ||
| "min_passage_size": None, | ||
| "planar_face_tolerance": None, | ||
| "seed_points": None, | ||
| } |
There was a problem hiding this comment.
In _get_gai_setting_whitelist, the new defaults whitelist entry seed_points is added, but the function still directly reads input_params.meshing.volume_zones earlier to detect rotation zones. That will raise AttributeError if Geometry AI is enabled while using ModularMeshingWorkflow (which uses zones instead). Consider switching that rotation-zone detection to use getattr(..., "volume_zones", None) with a fallback to zones (similar to _get_gai_seed_points) so GAI filtering works for both schemas.
| """ | ||
| Represents a separate zone in the mesh, defined by a point inside it. | ||
| Represents a separate zone in the mesh, defined by one or more interior seed points. | ||
| To be used only with snappyHexMesh. |
There was a problem hiding this comment.
The SeedpointVolume docstring says "To be used only with snappyHexMesh", but the validators now explicitly allow SeedpointVolume with the beta mesher as well. Please update the docstring to match the supported meshers (snappyHexMeshing or beta mesher) to avoid misleading API users.
| To be used only with snappyHexMesh. | |
| To be used with snappyHexMeshing or the beta mesher. |
| @contextual_model_validator(mode="after") | ||
| def _check_seedpoint_volume_usage(self, param_info: ParamsValidationInfo): | ||
| """Validate SeedpointVolume usage in modular meshing schema.""" | ||
| _validate_seedpoint_volume_usage(_collect_all_seedpoint_volumes(self.zones), param_info) | ||
| return self |
There was a problem hiding this comment.
With the updated SeedpointVolume capability checks, consider also updating the existing snappy-specific error message in _check_snappy_zones that says snappy zones are "not CustomZones"—the actual conflict is between SeedpointVolume and CustomVolume entities inside CustomZones, so the message is currently misleading.
piotrkluba
left a comment
piotrkluba
left a comment
There was a problem hiding this comment.
LGTM, make sure to run localtests (snappyMeshing, snappyMeshingAutomatedFarfield, snappyMeshingMultiZone) with this client to make sure everything is as expected e2e
Support multi-point SeedpointVolume across volume/surface translators and GAI seed points
Summary
• Extends SeedpointVolume to accept one-or-many point_in_mesh points in the Python model.
• Updates volume meshing translation to emit seed-based custom zones as seedPoints (list of
points) for VolumeMesher.
• Adds Snappy translation-time enforcement that each SeedpointVolume provides exactly one
point for locationInMesh, with clear error messaging for multi-point cases.
• Aligns meshing validation so SeedpointVolume is allowed only when using snappyHexMeshing or
the beta mesher, across both MeshingParams.volume_zones and ModularMeshingWorkflow.zones.
• Adds GAI path support by populating simulationJson -> meshing -> defaults -> seed_points
from SeedpointVolume entries, matching SurfaceMesher.cpp expectations.
• Expands translator/validation tests to cover:
• multi-point seedpoint translation for volume meshing,
• Snappy rejection of multi-point locationInMesh,
• GAI defaults.seed_points emission,
• existing GAI and surface translator behavior regression checks.
Files changed (vs
Flow360main)• flow360/component/simulation/primitives.py
• flow360/component/simulation/meshing_param/params.py
• flow360/component/simulation/translator/volume_meshing_translator.py
• flow360/component/simulation/translator/surface_meshing_translator.py
• tests/simulation/params/meshing_validation/test_meshing_param_validation.py
• tests/simulation/translator/test_volume_meshing_translator.py
• tests/simulation/translator/test_surface_meshing_translator.py
Test plan
• poetry run pytest
tests/simulation/params/meshing_validation/test_meshing_param_validation.py
• poetry run pytest tests/simulation/translator/test_volume_meshing_translator.py
• poetry run pytest tests/simulation/translator/test_surface_meshing_translator.py
Note
Medium Risk
Changes the
SeedpointVolumeschema and meshing JSON output (pointInMesh->seedPoints/seed_points), which can break downstream consumers and requires careful compatibility with snappy vs beta/inhouse meshers.Overview
Adds support for multi-point
SeedpointVolumeby allowingpoint_in_meshto accept either a single point or a list of points (normalized toList[Point]).Updates translation/validation to match new contracts: volume meshing now emits
seedPoints: [[x,y,z], ...]for seedpoint zones, snappy surface meshing enforces exactly one seed point per zone when generatinglocationInMesh, and GAI surface-mesher filtering now populatesmeshing.defaults.seed_pointsfrom allSeedpointVolumepoints.Adds consistent validation that
SeedpointVolumeis only permitted withsnappyHexMeshingor the beta mesher, plus new/updated tests covering the new JSON shape, multi-point behavior, and snappy rejection of multiple points.Reviewed by Cursor Bugbot for commit 3413d3c. Bugbot is set up for automated code reviews on this repo. Configure here.