Skip to content

Conversation

@weiliangjin2021
Copy link
Collaborator

@weiliangjin2021 weiliangjin2021 commented Jan 28, 2026

Note

Medium Risk
Adds a new core geometry primitive and integrates it into serialization and geometry traversal/flattening paths; mistakes could affect simulation setup or geometry processing for users adopting the new type.

Overview
Introduces GeometryArray, a new Geometry type for representing many copies of a base geometry using per-instance offsets and optional 4x4 transforms, plus a convenience constructor Geometry.array(offsets=..., transforms=...).

Wires the new type through public exports (tidy3d.__init__), geometry utilities (flatten_groups/traverse_geometries/validate_no_transformed_polyslabs), and JSON schemas so simulations/components can serialize/deserialize GeometryArray. Adds docs and comprehensive tests for validation, bounds/inside behavior, plane intersections, volume/surface area behavior, and equivalence to GeometryGroup, along with shared validators for finite-geometry and invertible transforms.

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

Greptile Overview

Greptile Summary

This PR introduces GeometryArray, a new class for efficiently representing repeated geometry instances with optional transformations and translations. The implementation includes a convenience method geometry.array() available on all geometry objects.

Key Changes

  • New GeometryArray class: Stores a single base geometry with arrays of offsets and/or transformation matrices, creating transformed instances on-demand via cached properties
  • Helper validators: Extracted assert_geometry_finite() and check_transform_invertible() validators to avoid code duplication between Transformed and GeometryArray
  • Utility function updates: Added GeometryArray support in flatten_groups(), traverse_geometries(), and validate_no_transformed_polyslabs()
  • Comprehensive testing: Added 20+ test cases covering creation, validation, bounds, inside checks, transforms, plotting, and equivalence with GeometryGroup
  • Documentation: Added dedicated section in API docs with usage examples

Implementation Details

The class efficiently handles three modes:

  1. Offsets only: Pure translations of the base geometry
  2. Transforms only: Rotations/reflections at the origin
  3. Both: Transforms applied before translation offsets

The implementation internally creates Transformed geometries (cached) and delegates most operations to them, ensuring consistency with existing geometry behavior.

Confidence Score: 4/5

  • Safe to merge with minor docstring formatting fixes
  • High-quality implementation with excellent test coverage and documentation. Two minor syntax issues found with docstring formatting (backticks should be doubled for RST). Core logic is sound with proper validation, caching, and integration with existing utilities.
  • Pay attention to tidy3d/components/geometry/base.py for the docstring syntax corrections

Important Files Changed

Filename Overview
tests/test_components/test_geometry.py Added comprehensive test coverage for GeometryArray with 20+ test cases
tidy3d/components/geometry/base.py Implemented GeometryArray class with helper validators and Geometry.array() method; minor issue with validation message formatting
tidy3d/components/geometry/utils.py Added GeometryArray support in utility functions (flatten_groups, traverse_geometries, validate_no_transformed_polyslabs)

Sequence Diagram

sequenceDiagram
    participant User
    participant Geometry
    participant GeometryArray
    participant Transformed
    participant Structure

    User->>Geometry: Create base geometry (e.g., Box)
    User->>Geometry: Call geometry.array(offsets, transforms)
    Geometry->>GeometryArray: Create GeometryArray instance
    GeometryArray->>GeometryArray: Validate offsets and transforms
    GeometryArray->>GeometryArray: Check transforms are invertible
    GeometryArray->>GeometryArray: Validate geometry has finite bounds
    GeometryArray-->>User: Return GeometryArray instance
    
    User->>Structure: Create Structure with GeometryArray
    Structure->>GeometryArray: Query bounds
    GeometryArray->>GeometryArray: Compute _get_full_transform() for each instance
    GeometryArray->>Transformed: Create Transformed geometries (cached)
    Transformed-->>GeometryArray: Return transformed instances
    GeometryArray->>GeometryArray: Compute combined bounds
    GeometryArray-->>Structure: Return bounds
    
    User->>GeometryArray: Call inside(x, y, z)
    GeometryArray->>Transformed: Check inside() for each instance
    Transformed-->>GeometryArray: Return boolean arrays
    GeometryArray->>GeometryArray: OR results together
    GeometryArray-->>User: Return combined result
Loading

@weiliangjin2021 weiliangjin2021 added the awaiting backend not to be merged as backend is not finalized label Jan 28, 2026
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

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.

@weiliangjin2021
Copy link
Collaborator Author

Note: I haven't reviewed tests myself yet, so please skip reviewing tests for now

@github-actions
Copy link
Contributor

github-actions bot commented Jan 28, 2026

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

  • tidy3d/init.py (100%)
  • tidy3d/components/geometry/base.py (92.8%): Missing lines 139,3847,3904,3958,3989-3990,3999
  • tidy3d/components/geometry/utils.py (37.5%): Missing lines 251,286,419-421

Summary

  • Total: 107 lines
  • Missing: 12 lines
  • Coverage: 88%

tidy3d/components/geometry/base.py

Lines 135-143

  135         _ = np.linalg.inv(transform)
  136     except np.linalg.LinAlgError as err:
  137         if index is not None:
  138             raise ValidationError(f"Transform at index {index} is not invertible.") from err
! 139         raise ValidationError("Transform matrix is not invertible.") from err
  140 
  141 
  142 class Geometry(Tidy3dBaseModel, ABC):
  143     """Abstract base class, defines where something exists in space."""

Lines 3843-3851

  3843         return np.matmul(translations, transforms)
  3844 
  3845     def _get_full_transform(self, index: int) -> MatrixReal4x4:
  3846         """Get the full 4x4 transform for a geometry at given index (transform + translation)."""
! 3847         return self._all_transforms[index]
  3848 
  3849     @cached_property
  3850     def _transformed_geometries(self) -> list[Transformed]:
  3851         """List of transformed geometries in the array."""

Lines 3900-3908

  3900             List of 2D shapes that intersect plane.
  3901             For more details refer to
  3902             `Shapely's Documentation <https://shapely.readthedocs.io/en/stable/project.html>`_.
  3903         """
! 3904         return self._geometry_group.intersections_tilted_plane(
  3905             normal, origin, to_2D, cleanup=cleanup, quad_segs=quad_segs
  3906         )
  3907 
  3908     def intersections_plane(

Lines 3954-3962

  3954         -------
  3955         bool
  3956             Whether this geometry intersects the plane.
  3957         """
! 3958         return self._geometry_group.intersects_axis_position(axis, position)
  3959 
  3960     def inside(self, x: NDArray[float], y: NDArray[float], z: NDArray[float]) -> NDArray[bool]:
  3961         """For input arrays ``x``, ``y``, ``z`` of arbitrary but identical shape, return an array
  3962         with the same shape which is ``True`` for every point in zip(x, y, z) that is inside the

Lines 3985-3994

  3985     def _surface_area(self, bounds: Bound) -> float:
  3986         """Returns object's surface area within given bounds."""
  3987         # Surface area cannot be reliably computed when non-trivial transforms are present
  3988         if self.transforms is not None:
! 3989             log.warning("Surface area of transformed elements cannot be calculated.")
! 3990             return None
  3991         # For pure translations, sum surface areas using local bounds for base geometry
  3992         total_area = 0.0
  3993         for geom in self._transformed_geometries:
  3994             # Transform bounds to local coordinate system

Lines 3995-4003

  3995             vertices = np.dot(geom.inverse, Transformed._vertices_from_bounds(bounds))[:3]
  3996             local_bounds = (tuple(vertices.min(axis=1)), tuple(vertices.max(axis=1)))
  3997             instance_area = self.geometry.surface_area(local_bounds)
  3998             if instance_area is None:
! 3999                 return None
  4000             total_area += instance_area
  4001         return total_area
  4002 

tidy3d/components/geometry/utils.py

Lines 247-255

  247                 flatten_array=flatten_array,
  248                 transform=new_transform,
  249             )
  250         elif flatten_array and isinstance(geometry, base.GeometryArray):
! 251             yield from flatten_groups(
  252                 *geometry._transformed_geometries,
  253                 flatten_nonunion_type=flatten_nonunion_type,
  254                 flatten_transformed=flatten_transformed,
  255                 flatten_array=flatten_array,

Lines 282-290

  282     elif isinstance(geometry, base.ClipOperation):
  283         yield from traverse_geometries(geometry.geometry_a)
  284         yield from traverse_geometries(geometry.geometry_b)
  285     elif isinstance(geometry, base.GeometryArray):
! 286         yield from traverse_geometries(geometry.geometry)
  287     yield geometry
  288 
  289 
  290 def from_shapely(

Lines 415-425

  415         validate_no_transformed_polyslabs(geometry.geometry_a, transform)
  416         validate_no_transformed_polyslabs(geometry.geometry_b, transform)
  417     elif isinstance(geometry, base.GeometryArray):
  418         # For GeometryArray, check each instance's transform combined with the base geometry
! 419         for i in range(geometry.num_geometries):
! 420             instance_transform = np.dot(transform, geometry._get_full_transform(i))
! 421             validate_no_transformed_polyslabs(geometry.geometry, instance_transform)
  422 
  423 
  424 class SnapLocation(Enum):
  425     """Describes different methods for defining the snapping locations."""

@weiliangjin2021 weiliangjin2021 force-pushed the weiliang/FXC-5105-geometry-array-feature-for-repeated-shapes branch 2 times, most recently from f2905bf to c50e72c Compare January 29, 2026 19:37
…tances

Add GeometryArray class that enables efficient representation of repeated
geometric patterns using arrays of offsets and optional transformations.
@weiliangjin2021 weiliangjin2021 force-pushed the weiliang/FXC-5105-geometry-array-feature-for-repeated-shapes branch from c50e72c to a7bfa3d Compare January 29, 2026 19:40
@yaugenst-flex
Copy link
Collaborator

We‘ll need to consider adjoint support for this too @groberts-flex

@groberts-flex
Copy link
Contributor

Great, thanks for the heads up! I'll take a look through and add this to the roadmap.

# translation matrix: [[1,0,0,x], [0,1,0,y], [0,0,1,z], [0,0,0,1]]
translations = np.broadcast_to(Transformed.identity(), shape).copy()
if self.offsets is not None:
translations[:, :3, 3] = self.offsets
Copy link
Contributor

Choose a reason for hiding this comment

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

We would just ignore the offset from self.transforms? Or do we want to forbid them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch. it's a bug here

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

Labels

awaiting backend not to be merged as backend is not finalized

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants