-
Notifications
You must be signed in to change notification settings - Fork 9
[FXC-4069] Allow AutomatedFarfield with CustomVolume #1800
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
735dd31 to
acb9a32
Compare
There was a problem hiding this 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 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| # After expansion, GhostSurface becomes GhostSphere/GhostCircularPlane. | ||
| if surface.name == "farfield": | ||
| zone_name = "farfield" | ||
| continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ghost surface filter unintentionally catches WindTunnelGhostSurface via inheritance
Medium Severity
WindTunnelGhostSurface inherits from GhostSurface, so the isinstance(surface, (GhostSurface, GhostSphere, GhostCircularPlane)) check inadvertently matches WindTunnelGhostSurface instances too. This causes wind tunnel ghost surface boundaries (e.g., windTunnelLeft, windTunnelRight) to be silently dropped from the patches list when a CustomVolume uses them as boundaries with WindTunnelFarfield. Previously, all boundary names were included in patches. The comment correctly says "AutomatedFarfield ghost entities," but the code is broader than intended due to inheritance.
| # AutomatedFarfield.farfield, so the translator can identify which zone is the exterior farfield zone. | ||
| has_automated_farfield = any(isinstance(z, AutomatedFarfield) for z in v) | ||
| has_custom_zones = any(isinstance(z, CustomZones) for z in v) | ||
| if has_automated_farfield and has_custom_zones: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validator triggers incorrectly for CustomZones without CustomVolume
Low Severity
The guard condition has_custom_zones checks whether any CustomZones exists, but the comment and intent say "If AutomatedFarfield is used with CustomVolume(s)." If CustomZones contains only SeedpointVolume entities (no CustomVolume), the inner loop finds no matching CustomVolume, farfield_custom_volumes stays empty, and the validator raises a spurious error requiring a CustomVolume with the farfield ghost — even though there are no CustomVolume entities to validate. The condition needs to check for the presence of actual CustomVolume entities, not just CustomZones.


user must specify exactly one outer CustomVolume using AutomatedFarfield and exterior geometry surfaces
still buggy (cursor comments and beyond), needs fixing
Note
Medium Risk
Touches meshing validation and translation logic around farfield zone identification; misconfiguration could lead to wrong zone naming/patch lists and mesher failures, but changes are scoped and covered by new tests.
Overview
Allows
AutomatedFarfieldto be combined withCustomZonesby introducing a farfield ghost boundary marker on exactly oneCustomVolumeto identify the exterior zone.Validation now enforces that exactly one
CustomVolumeincludesAutomatedFarfield.farfield(or its expandedGhostSphere/GhostCircularPlaneforms), andCustomVolumeis treated as compatible with theautofarfield method when using the beta mesher.The volume-meshing translator skips ghost farfield entities from the
patcheslist and renames the marked exterior custom volume tofarfieldin the translatedzonesoutput; tests were updated/added to cover these scenarios and error cases.Written by Cursor Bugbot for commit acb9a32. This will update automatically on new commits. Configure here.