Skip to content

FXC-2087 Implement automatic structure extrusion for boundary waveports #2793

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 20, 2025
Merged

FXC-2087 Implement automatic structure extrusion for boundary waveports #2793

merged 1 commit into from
Oct 20, 2025

Conversation

@George-Guryev-flxcmp
Copy link
Contributor

@George-Guryev-flxcmp George-Guryev-flxcmp commented Sep 4, 2025 •
edited by greptile-apps bot
Loading

This PR adds automatic extrusion of structures intersecting waveports that are placed on boundaries.

Key changes:

  • Introduced the field in to mark ports for which material should be extruded.
  • Implemented the extrusion logic in , ensuring mode sources, internal absorbers, and PEC frames are fully contained within the extruded structures.
  • Extrusion occurs only when is set to .

This improves simulation accuracy when waveports are located on structure boundaries and simplifies user workflow.

Greptile Summary

Updated On: 2025-09-04 15:39:10 UTC

This PR implements automatic structure extrusion for waveports positioned on simulation boundaries. The main functionality adds a new extrude_structures boolean field to the WavePort class that, when set to True, triggers automatic extension of intersecting structures to ensure mode sources, internal absorbers, and PEC frames are fully contained within the material geometry.

The core implementation resides in the TerminalComponentModeler class with the addition of a new _extrude_port_structures method that performs the geometric operations. This method identifies structures intersecting the waveport plane and creates extended PolySlab geometries that stretch from the port location to cover all necessary simulation components. The extrusion logic handles complex geometries with holes using ClipOperation and integrates seamlessly into the existing simulation pipeline.

This feature addresses a common simulation accuracy issue where waveports placed directly on structure boundaries might not provide adequate material coverage for electromagnetic field calculations. By automating the extrusion process, the PR simplifies user workflow while ensuring proper containment of all port-related simulation elements.

Important Files Changed

Changed Files
Filename Score Overview
tidy3d/plugins/smatrix/component_modelers/terminal.py 3/5 Implements core extrusion logic with potential bounds checking issues
tidy3d/plugins/smatrix/ports/wave.py 5/5 Clean addition of extrude_structures boolean field with proper defaults
tests/test_plugins/smatrix/test_terminal_component_modeler.py 4/5 Comprehensive test validating extrusion functionality for coaxial geometry
CHANGELOG.md 4/5 Documents new feature with clear user-facing descriptions

Confidence score: 3/5

  • This PR requires careful review due to potential runtime errors in the extrusion logic
  • Score lowered due to unsafe array access without bounds checking and complex geometric operations that could fail
  • Pay close attention to tidy3d/plugins/smatrix/component_modelers/terminal.py, specifically the _extrude_port_structures method

Sequence Diagram

sequenceDiagram
    participant User
    participant TerminalComponentModeler as TCM
    participant WavePort
    participant Simulation as Sim
    participant Structure
    participant GeometryGroup as GeoGroup
    participant PolySlab
    participant ClipOperation as ClipOp

    User->>WavePort: "Create WavePort with extrude_structures=True"
    User->>TCM: "Create TerminalComponentModeler with WavePort"
    
    User->>TCM: "Access sim_dict property"
    TCM->>TCM: "_add_source_to_sim(network_index)"
    TCM->>WavePort: "to_source(source_time, snap_center)"
    WavePort-->>TCM: "ModeSource"
    TCM->>Sim: "updated_copy(sources=[port_source])"
    Sim-->>TCM: "simulation with source"
    
    TCM->>TCM: "_extrude_port_structures(sim_with_src)"
    
    loop "For each WavePort with extrude_structures=True"
        TCM->>WavePort: "Check extrude_structures flag"
        WavePort-->>TCM: "True"
        
        TCM->>TCM: "Calculate mode source position and offset"
        TCM->>TCM: "Define extrusion bounding box"
        TCM->>TCM: "Get slice planes (left and right)"
        
        loop "For each structure in simulation"
            TCM->>Structure: "Get geometry intersections with slice planes"
            Structure-->>TCM: "Shapely polygons"
            
            loop "For each intersecting polygon"
                TCM->>PolySlab: "Create outer shell from exterior vertices"
                PolySlab-->>TCM: "Outer PolySlab"
                
                alt "Polygon has holes"
                    loop "For each hole"
                        TCM->>PolySlab: "Create hole PolySlab from interior vertices"
                        PolySlab-->>TCM: "Hole PolySlab"
                    end
                    TCM->>GeoGroup: "Create GeometryGroup(hole_polyslabs)"
                    GeoGroup-->>TCM: "Holes geometry group"
                    TCM->>ClipOp: "ClipOperation(difference, outer_shell, holes)"
                    ClipOp-->>TCM: "Extruded geometry with holes"
                else "No holes"
                    TCM->>TCM: "Use outer shell as extruded geometry"
                end
                
                TCM->>TCM: "Add extruded geometry to new_geoms list"
            end
            
            TCM->>TCM: "Add original structure geometry to new_geoms"
            TCM->>GeoGroup: "Create GeometryGroup(new_geoms)"
            GeoGroup-->>TCM: "Combined geometry group"
            TCM->>Structure: "Create new Structure with combined geometry"
            Structure-->>TCM: "Extended structure"
            TCM->>TCM: "Add to new_structures list"
        end
    end
    
    TCM->>Sim: "updated_copy(structures=new_structures)"
    Sim-->>TCM: "Simulation with extruded structures"
    TCM-->>User: "SimulationMap with extruded structures"
Loading

greptile-apps[bot]
greptile-apps bot reviewed Sep 4, 2025
View reviewed changes
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.

4 files reviewed, 2 comments

Edit Code Review Bot Settings | Greptile

@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/waveport branch 2 times, most recently from f844379 to 8fcbe0b Compare September 4, 2025 15:48
daquinteroflex
daquinteroflex reviewed Sep 4, 2025
View reviewed changes
@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2025 •
edited
Loading

Diff Coverage

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

  • tidy3d/components/geometry/utils.py (100%)
  • tidy3d/components/simulation.py (94.4%): Missing lines 2165
  • tidy3d/plugins/smatrix/component_modelers/terminal.py (100%)
  • tidy3d/plugins/smatrix/ports/wave.py (100%)

Summary

  • Total: 74 lines
  • Missing: 1 line
  • Coverage: 98%

tidy3d/components/simulation.py

Lines 2161-2169

  2161         else:
  2162             if direction == "-":
  2163                 del surfaces[2 * axis + 1]
  2164             else:
! 2165                 del surfaces[2 * axis]
  2166 
  2167         structure = Structure(
  2168             geometry=GeometryGroup(
  2169                 geometries=surfaces,

dbochkov-flexcompute
dbochkov-flexcompute reviewed Sep 4, 2025
View reviewed changes
@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/waveport branch 2 times, most recently from a469ec2 to f1007a7 Compare September 5, 2025 00:46
@dmarek-flex
Copy link
Contributor

dmarek-flex commented Sep 5, 2025

Looking good, just testing it on a simple example though and I think I found a bug. In this 2 waveport example, the extrusion works properly, but the final list of structures is out of order. In this case, the extended substrate is added later in the list which overlaps previously extended strips.

image

extrude_stripline.ipynb

@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/waveport branch 2 times, most recently from 67a0ba1 to 102b073 Compare September 8, 2025 21:12
dbochkov-flexcompute
dbochkov-flexcompute reviewed Sep 9, 2025
View reviewed changes
Copy link
Contributor

@dbochkov-flexcompute dbochkov-flexcompute left a comment

Choose a reason for hiding this comment

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

Almost there, a couple of comments + let's add an example where a waveguide assembly contains a geometry which is a union of two disjoint objects (something like GeometryGroup(geometries=[box_left, box_right]), like a differential strip

@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/waveport branch 2 times, most recently from 7e11342 to 0d3abc3 Compare September 10, 2025 04:09
dmarek-flex
dmarek-flex approved these changes Sep 10, 2025
View reviewed changes
Copy link
Contributor

@dmarek-flex dmarek-flex left a comment

Choose a reason for hiding this comment

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

Thanks! Looking like a good feature, just a couple of minor style comments.

@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/waveport branch 4 times, most recently from c01d9fa to 156ae24 Compare September 10, 2025 19:35
daquinteroflex
daquinteroflex approved these changes Oct 1, 2025
View reviewed changes
Copy link
Collaborator

@daquinteroflex daquinteroflex left a comment

Choose a reason for hiding this comment

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

Looks good!

@George-Guryev-flxcmp George-Guryev-flxcmp changed the title Implement automatic structure extrusion for boundary waveports FXC-2087 Implement automatic structure extrusion for boundary waveports Oct 15, 2025
@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/waveport branch 2 times, most recently from a80a406 to 9f9b5ba Compare October 15, 2025 19:37
@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/waveport branch 4 times, most recently from 5b4f4b6 to 7e4bcb7 Compare October 17, 2025 19:47
dbochkov-flexcompute
dbochkov-flexcompute requested changes Oct 17, 2025
View reviewed changes
@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/waveport branch 4 times, most recently from 484720b to 696fcc1 Compare October 20, 2025 18:16
@George-Guryev-flxcmp George-Guryev-flxcmp added this pull request to the merge queue Oct 20, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 20, 2025
@George-Guryev-flxcmp George-Guryev-flxcmp added this pull request to the merge queue Oct 20, 2025
Merged via the queue into develop with commit 94b6267 Oct 20, 2025
25 checks passed
@George-Guryev-flxcmp George-Guryev-flxcmp deleted the george/waveport branch October 20, 2025 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dbochkov-flexcompute dbochkov-flexcompute dbochkov-flexcompute approved these changes

@daquinteroflex daquinteroflex daquinteroflex approved these changes

@dmarek-flex dmarek-flex dmarek-flex approved these changes

+1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@George-Guryev-flxcmp @dmarek-flex @dbochkov-flexcompute @daquinteroflex