Skip to content

Conversation

@dmarek-flex
Copy link
Contributor

@dmarek-flex dmarek-flex commented Jan 5, 2026

intersections_plane for PolySlab was missing intersections with planes that were coincident on the + side of polyslab side walls.

Greptile Summary

This PR fixes a bug in PolySlab.intersections_plane where intersections were missed when a plane was coincident with the positive side of PolySlab side faces. The fix replaces asymmetric inequality checks (<= and >) with symmetric strict inequalities plus explicit handling of edges that touch the plane.

Key Changes:

  • Replaced directional inequality logic with clear separation between strictly crossing edges and touching edges
  • Uses XOR logic to detect edges where exactly one vertex is on the plane, excluding edges that lie entirely on the plane
  • Added explanatory comments describing the intersection detection strategy
  • Test coverage for all four sides of a rectangular PolySlab with coincident planes
  • Proper changelog entry under Fixed section

Technical Details:
The old logic used f <= position AND orig > position which would miss edges starting on the plane and extending in the negative direction. The new logic explicitly includes all edges where exactly one endpoint is on the plane via touches = np.logical_xor(orig_on_plane, f_on_plane), ensuring symmetric handling of both directions.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The fix is well-reasoned and addresses a clear geometric edge case. The logic correctly handles floating-point comparisons using np.isclose, and the XOR-based touching detection elegantly excludes edges lying on the plane while including edges with one endpoint on the plane. The change automatically propagates to the slanted case via method reuse. Test coverage is adequate and the implementation follows project conventions.
  • No files require special attention

Important Files Changed

Filename Overview
tidy3d/components/geometry/polyslab.py Fixed edge intersection detection logic to handle planes coincident with side faces using XOR-based touching detection and clear comments
tests/test_components/test_geometry.py Added test for coincident plane intersections covering all four sides of a rectangular PolySlab; could verify actual intersection shapes
CHANGELOG.md Clear and properly formatted changelog entry under Fixed section with appropriate code identifier formatting

Sequence Diagram

sequenceDiagram
    participant User
    participant PolySlab
    participant IntersectionMethod as _find_intersecting_ys_angle_vertical
    participant NPUtils as numpy operations
    
    User->>PolySlab: intersections_plane(x=500)
    PolySlab->>IntersectionMethod: _find_intersecting_ys_angle_vertical(vertices, 500, axis=0)
    
    Note over IntersectionMethod: Process vertices for axis orientation
    IntersectionMethod->>IntersectionMethod: Roll vertices to get forward edges
    
    Note over IntersectionMethod: Detect intersection types
    IntersectionMethod->>NPUtils: np.isclose(x_vertices_axis, position)
    NPUtils-->>IntersectionMethod: orig_on_plane (boolean array)
    IntersectionMethod->>NPUtils: np.isclose(x_vertices_f, position)
    NPUtils-->>IntersectionMethod: f_on_plane (boolean array)
    
    Note over IntersectionMethod: NEW: Calculate touching edges
    IntersectionMethod->>NPUtils: np.logical_xor(orig_on_plane, f_on_plane)
    NPUtils-->>IntersectionMethod: touches (edges with one vertex on plane)
    
    Note over IntersectionMethod: Calculate crossing edges
    IntersectionMethod->>IntersectionMethod: crosses_b = (orig > pos) & (f < pos)
    IntersectionMethod->>IntersectionMethod: crosses_f = (orig < pos) & (f > pos)
    
    Note over IntersectionMethod: Combine intersection types
    IntersectionMethod->>IntersectionMethod: intersects_b = crosses_b | touches
    IntersectionMethod->>IntersectionMethod: intersects_f = crosses_f | touches
    
    Note over IntersectionMethod: Calculate intersection points
    IntersectionMethod->>IntersectionMethod: For each intersecting segment:<br/>compute slope and y-coordinate
    
    IntersectionMethod-->>PolySlab: ints_y_sort, ints_angle_sort
    PolySlab-->>User: List of intersection shapes
Loading

Note

Addresses missed intersections when a slicing plane is coincident with PolySlab side faces.

  • Reworks intersections_plane edge tests to use symmetric strict crossings plus explicit single-endpoint-on-plane detection (excludes whole-edge-on-plane); respects exclude_on_vertices
  • Deduplicates intersection points and restores degenerate pairs for tangent touches to ensure correct polygon construction
  • Adds tests covering coincident planes on all sides of a rectangular PolySlab and a rotated-square (diamond) case, including tangent-touch degeneracies
  • Updates CHANGELOG.md under Fixed to note the PolySlab.intersections_plane bugfix

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

@dmarek-flex dmarek-flex self-assigned this Jan 5, 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, 1 comment

Edit Code Review Agent Settings | Greptile

@dmarek-flex dmarek-flex force-pushed the dmarek/FXC-4688-Incorrect-intersection-of-polyslab-with-coincident-plane branch from f0a7282 to 54ae5af Compare January 5, 2026 20:22
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026

Diff Coverage

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

  • tidy3d/components/geometry/polyslab.py (100%)

Summary

  • Total: 23 lines
  • Missing: 0 lines
  • Coverage: 100%

@dmarek-flex dmarek-flex force-pushed the dmarek/FXC-4688-Incorrect-intersection-of-polyslab-with-coincident-plane branch 3 times, most recently from 71f09f4 to 6204e40 Compare January 6, 2026 21:02
@dmarek-flex dmarek-flex force-pushed the dmarek/FXC-4688-Incorrect-intersection-of-polyslab-with-coincident-plane branch 2 times, most recently from 8843177 to 56aac41 Compare January 9, 2026 18:27
@dmarek-flex
Copy link
Contributor Author

@lucas-flexcompute Does this look good?

…plane is coincident with PolySlab side faces
@dmarek-flex dmarek-flex force-pushed the dmarek/FXC-4688-Incorrect-intersection-of-polyslab-with-coincident-plane branch from f24ee67 to a12f5a5 Compare January 9, 2026 19:41
@dmarek-flex dmarek-flex enabled auto-merge January 9, 2026 19:41
@dmarek-flex dmarek-flex added this pull request to the merge queue Jan 9, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 9, 2026
@dmarek-flex dmarek-flex added this pull request to the merge queue Jan 9, 2026
Merged via the queue into develop with commit f614a6e Jan 9, 2026
45 of 53 checks passed
@dmarek-flex dmarek-flex deleted the dmarek/FXC-4688-Incorrect-intersection-of-polyslab-with-coincident-plane branch January 9, 2026 21:53
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.

3 participants