-
Notifications
You must be signed in to change notification settings - Fork 70
feat(FXC-5154) Warn gmsh min cylinder radii #3217
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: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,8 @@ | |
| StructureStructureInterface, | ||
| ) | ||
| from tidy3d.components.geometry.base import Box | ||
| from tidy3d.components.geometry.primitives import Cylinder | ||
| from tidy3d.components.geometry.utils import traverse_geometries | ||
| from tidy3d.components.material.tcad.charge import ( | ||
| ChargeConductorMedium, | ||
| SemiconductorMedium, | ||
|
|
@@ -127,6 +129,11 @@ | |
| # define some limits for transient heat simulations | ||
| TRANSIENT_HEAT_MAX_STEPS = 1000 | ||
|
|
||
| # OpenCASCADE minimum tolerance for cylinder radii | ||
| OPENCASCADE_CYLINDER_RADIUS_TOL = 1e-6 | ||
| # Minimum radius as fraction of the larger radius (for tapered cylinders) | ||
| MIN_CYLINDER_RADIUS_FRACTION = 0.01 | ||
|
|
||
|
|
||
| class TCADAnalysisTypes(str, Enum): | ||
| """Enumeration of the types of simulations currently supported""" | ||
|
|
@@ -363,6 +370,45 @@ def check_unsupported_geometries(cls, val): | |
| ) | ||
| return val | ||
|
|
||
| @pd.validator("structures", always=True) | ||
| def _warn_small_cylinder_radius(cls, val): | ||
marc-flex marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| """Warn if any Cylinder geometry has radius too small for meshing.""" | ||
| for structure in val: | ||
| for geometry in traverse_geometries(structure.geometry): | ||
| if isinstance(geometry, Cylinder): | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| r_bottom = geometry.radius_bottom | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| r_top = geometry.radius_top | ||
| is_tapered = not np.isclose(r_bottom, r_top) | ||
|
|
||
| # Compute minimum allowed radius (matches backend heat_mesh.py logic) | ||
| min_radius = max( | ||
| OPENCASCADE_CYLINDER_RADIUS_TOL, | ||
| MIN_CYLINDER_RADIUS_FRACTION * max(abs(r_bottom), abs(r_top)), | ||
| ) | ||
|
|
||
| # Warn if radii are below minimum | ||
| if is_tapered: | ||
| if r_bottom < min_radius: | ||
| log.warning( | ||
| f"Cylinder 'radius_bottom' ({r_bottom:.3e}) is below the minimum " | ||
| f"radius for meshing ({min_radius:.3e}). The sidewall angle may be " | ||
| f"too steep. Will be clamped to minimum radius or mesh size, whichever is larger." | ||
| ) | ||
| if r_top < min_radius: | ||
| log.warning( | ||
| f"Cylinder 'radius_top' ({r_top:.3e}) is below the minimum " | ||
| f"radius for meshing ({min_radius:.3e}). The sidewall angle may be " | ||
| f"too steep. Will be clamped to minimum radius or mesh size, whichever is larger." | ||
| ) | ||
| else: | ||
| if r_bottom < min_radius: | ||
| log.warning( | ||
| f"Cylinder 'radius' ({r_bottom:.3e}) is below the minimum " | ||
| f"radius for meshing ({min_radius:.3e}). " | ||
| f"Will be clamped to minimum radius or mesh size, whichever is larger." | ||
| ) | ||
| return val | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing CHANGELOG entry for user-facing featureLow Severity This PR adds a new user-facing feature (warning messages for small cylinder radii in |
||
|
|
||
| @staticmethod | ||
| def _check_cross_solids(objs: tuple[Box, ...], values: dict) -> tuple[int, ...]: | ||
| """Given model dictionary ``values``, check whether objects in list ``objs`` cross | ||
|
|
||


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.
I see, this will have side-effects on other uses of this function too, but this addition actually seems correct here, because a
Transformedgeometry could itself be a group or a clip operation, and previously we were not traversing those!