Skip to content

Conversation

@George-Guryev-flxcmp
Copy link
Contributor

@George-Guryev-flxcmp George-Guryev-flxcmp commented Nov 25, 2025

This PR adds feature for automatic extrusion of structures located next to the simulation boundaries into PML/Absorber. This is facilitated by additional field enable_extrusion in class AbsorberSpec.

Greptile Overview

Greptile Summary

This PR adds an API field enable_extrusion to the AbsorberSpec class, allowing users to enable automatic extrusion of structures into PML/Absorber boundary regions. The field is properly documented and tested.

Key Changes:

  • Added enable_extrusion boolean field to AbsorberSpec class (inherited by PML, StablePML, and Absorber)
  • Field defaults to False to maintain backward compatibility
  • Added comprehensive test coverage verifying the field works only on absorber types
  • Updated all simulation schemas to reflect the new field
  • Added clear changelog entry

Important Notes:

  • This appears to be an API-only addition; the actual implementation logic that uses this field is not present in this PR
  • The field is stored but not consumed by any simulation or validation logic yet
  • Similar to the extrude_structures field in WavePort, this likely prepares the groundwork for future implementation

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk as it only adds an API field without implementation logic
  • Score of 4 reflects that this is a clean API addition with proper documentation and tests, but the field is not yet used anywhere in the codebase. Minor style improvements suggested for test assertions. The change is backward compatible and follows established patterns.
  • No files require special attention - all changes are straightforward API additions

Important Files Changed

File Analysis

Filename Score Overview
tidy3d/components/boundary.py 5/5 Added enable_extrusion boolean field to AbsorberSpec class with proper default value and documentation
tests/test_components/test_boundaries.py 4/5 Added test for enable_extrusion field checking default values and exclusivity to absorber types; uses direct equality instead of is for boolean checks
CHANGELOG.md 5/5 Added clear user-facing changelog entry documenting the new enable_extrusion field in AbsorberSpec

Sequence Diagram

sequenceDiagram
    participant User
    participant Simulation
    participant BoundarySpec
    participant AbsorberSpec
    participant PML/Absorber
    
    User->>Simulation: Create simulation with boundary conditions
    User->>PML/Absorber: PML(enable_extrusion=True)
    PML/Absorber->>AbsorberSpec: Initialize with enable_extrusion field
    AbsorberSpec->>AbsorberSpec: Store enable_extrusion=True
    AbsorberSpec-->>PML/Absorber: Return configured boundary
    PML/Absorber-->>Simulation: Add to boundary specification
    Simulation->>BoundarySpec: Configure x/y/z boundaries
    
    Note over Simulation,BoundarySpec: Field is stored but not yet used in implementation
    Note over User,PML/Absorber: API surface prepared for future extrusion logic
Loading

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.

7 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2025

Diff Coverage

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

  • tidy3d/components/boundary.py (100%)

Summary

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

@daquinteroflex
Copy link
Collaborator

Are we sure this is not required for rc3? It's a schema change that will be blocked till 2.11. @George-Guryev-flxcmp @dbochkov-flexcompute @weiliangjin2021

@weiliangjin2021
Copy link
Collaborator

Are we sure this is not required for rc3? It's a schema change that will be blocked till 2.11. @George-Guryev-flxcmp @dbochkov-flexcompute @weiliangjin2021

If so, let's try to get it in for rc3. When is the deadline for merging PRs for rc3?

@daquinteroflex
Copy link
Collaborator

If so, let's try to get it in for rc3. When is the deadline for merging PRs for rc3?

Can you get this in for tomorrow?

@George-Guryev-flxcmp
Copy link
Contributor Author

If so, let's try to get it in for rc3. When is the deadline for merging PRs for rc3?

Can you get this in for tomorrow?

Sure thing.

@daquinteroflex daquinteroflex added rc3 3rd pre-release 2.10 and removed rc3 3rd pre-release 2.10 labels Nov 26, 2025
@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/extrusion_to_pml branch 4 times, most recently from 56baf28 to 98c56f5 Compare December 2, 2025 06:34
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.

5 participants