Skip to content

Comments

FaceGroup Specification of min Passage Size#1826

Open
NasserFlexCompute wants to merge 1 commit intomainfrom
nasser/faceBasedPassageSize
Open

FaceGroup Specification of min Passage Size#1826
NasserFlexCompute wants to merge 1 commit intomainfrom
nasser/faceBasedPassageSize

Conversation

@NasserFlexCompute
Copy link
Contributor

@NasserFlexCompute NasserFlexCompute commented Feb 22, 2026

Note

Low Risk
Small additive schema change plus fixture/test updates; limited to Geometry-AI meshing parameters and should be backward compatible.

Overview
Adds per-face-group min_passage_size support to GeometryRefinement, allowing local control over the hidden-geometry removal passage threshold (otherwise derived from geometry_accuracy/sealing_size).

Updates the GAI surface meshing translator reference JSON and the translator test to include and validate serialization of min_passage_size in refinement output.

Written by Cursor Bugbot for commit 7cff547. This will update automatically on new commits. Configure here.

Copilot AI review requested due to automatic review settings February 22, 2026 02:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for specifying min_passage_size at the face group level through the GeometryRefinement class. This allows users to override the global min_passage_size setting (from MeshingDefaults) for specific face groups, providing finer control over hidden geometry removal resolution.

Changes:

  • Added min_passage_size field to GeometryRefinement class for per-face-group specification
  • Updated test to include min_passage_size parameter in a GeometryRefinement instance
  • Updated reference JSON to reflect the expected serialization format

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
flow360/component/simulation/meshing_param/face_params.py Added min_passage_size as an optional field to GeometryRefinement class with appropriate type hints and description
tests/simulation/translator/test_surface_meshing_translator.py Added test coverage for the new min_passage_size field in a GeometryRefinement refinement
tests/simulation/translator/ref/surface_meshing/gai_surface_mesher.json Updated reference JSON to include the expected serialization of min_passage_size field

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

description="Minimum passage size that hidden geometry removal can resolve for this face group. "
"Internal regions connected by thin passages smaller than this size may not be detected. "
"If not specified, the value is derived from geometry_accuracy and sealing_size.",
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing validation for min_passage_size against remove_hidden_geometry

Medium Severity

The min_passage_size field on GeometryRefinement lacks the validation that exists at the defaults level in MeshingDefaults, where validate_min_passage_size_requires_remove_hidden_geometry ensures min_passage_size is only specified when remove_hidden_geometry is True. The field description explicitly ties it to "hidden geometry removal," but a user can set it on a face group without remove_hidden_geometry being enabled globally, leading to a silently ignored or inconsistently validated parameter.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant