Skip to content

Conversation

momchil-flex
Copy link
Collaborator

@momchil-flex momchil-flex commented Aug 8, 2025

Greptile Summary

This PR enhances the Tidy3D mode solver to support fully anisotropic materials by implementing proper grid positioning for off-diagonal permittivity tensor components and improving numerical robustness in symmetry detection.

The changes span three key areas:

  1. Epsilon Calculation Fix (simulation.py): Updates the grid coordinate handling for off-diagonal permittivity tensor components (like eps_xy, eps_xz, etc.). These components are now correctly positioned at grid boundaries with proper coordinate truncation, ensuring accurate electromagnetic field calculations on the Yee grid for anisotropic materials.

  2. Symmetry Detection Robustness (mode_solver.py): Replaces direct floating-point equality comparison with tolerance-based comparison using isclose() when checking if the mode plane center aligns with simulation symmetry center. This prevents numerical precision errors from incorrectly breaking symmetry assumptions.

  3. Tensorial Solver Enhancement (solver.py): Extends the mode solver with new parameters (dls for grid steps, dmin_pmc for PMC boundary conditions, and Nxy for grid dimensions) to enable full anisotropic mode solving capabilities. The code also includes organizational improvements by relocating utility methods.

These modifications work together to enable accurate mode solving for materials with full anisotropic permittivity and permeability tensors, which is critical for advanced photonic applications involving complex material properties.

Important Files Changed

Files Overview
Filename Score Overview
tidy3d/components/simulation.py 4/5 Fixed grid positioning for off-diagonal permittivity tensor components
tidy3d/components/mode/mode_solver.py 5/5 Replaced direct float comparison with tolerance-based isclose() for symmetry detection
tidy3d/components/mode/solver.py 2/5 Added anisotropic solver parameters but contains critical missing parameter bug

Confidence score: 3/5

  • This PR contains important enhancements for anisotropic mode solving but has a critical bug that could cause runtime errors
  • Score lowered due to missing col_mats parameter in the complex case of the tensorial solver, which could cause inconsistent behavior or crashes
  • Pay close attention to tidy3d/components/mode/solver.py where the col_mats parameter is missing in the complex solver call

Sequence Diagram

sequenceDiagram
    participant User
    participant ModeSolver
    participant EigSolver
    participant Simulation
    participant Grid
    participant Structure

    User->>ModeSolver: "Create mode solver with anisotropic materials"
    ModeSolver->>Simulation: "Get simulation structures and medium"
    Simulation->>Structure: "Check for fully anisotropic mediums"
    Structure-->>Simulation: "Return medium properties"
    Simulation-->>ModeSolver: "Return simulation data"
    
    User->>ModeSolver: "solve() - compute modes"
    ModeSolver->>ModeSolver: "_validate_rotate_structures()"
    alt angle_rotation enabled and angle_theta > 0
        ModeSolver->>ModeSolver: "_rotate_structures"
        ModeSolver->>Structure: "Rotate intersecting structures"
        Structure-->>ModeSolver: "Return rotated structures"
    end
    
    ModeSolver->>ModeSolver: "data_raw - get mode solver data"
    alt angle_rotation with bend
        ModeSolver->>ModeSolver: "rotated_mode_solver_data"
        ModeSolver->>ModeSolver: "rotated_structures_copy"
        ModeSolver->>ModeSolver: "_car_2_cyn() - convert to cylindrical"
        ModeSolver->>ModeSolver: "_mode_rotation() - rotate fields"
    else standard solve
        ModeSolver->>ModeSolver: "_data_on_yee_grid()"
    end
    
    ModeSolver->>Grid: "discretize_monitor() - get grid"
    Grid-->>ModeSolver: "Return discretized grid"
    
    ModeSolver->>EigSolver: "compute_modes()"
    EigSolver->>EigSolver: "format_medium_data() - process epsilon tensor"
    alt fully tensorial case
        EigSolver->>EigSolver: "solver_tensorial() - solve tensorial eigenvalue problem"
    else diagonal case  
        EigSolver->>EigSolver: "solver_diagonal() - solve diagonal eigenvalue problem"
    end
    EigSolver->>EigSolver: "solver_eigs() - eigenvalue solver"
    EigSolver-->>ModeSolver: "Return eigenvalues and eigenvectors"
    
    ModeSolver->>ModeSolver: "_postprocess_solver_fields() - convert to field components"
    alt colocate fields
        ModeSolver->>ModeSolver: "_colocate_data() - interpolate to grid boundaries"
    end
    ModeSolver->>ModeSolver: "_normalize_modes() - normalize field amplitudes"
    ModeSolver-->>User: "Return ModeSolverData with fields and effective indices"
Loading

Context used:

Rule - Use a tolerance-based comparison (e.g., np.isclose) for floating-point numbers instead of direct equality (==) to avoid precision issues. (link)

@momchil-flex momchil-flex marked this pull request as draft August 8, 2025 13:26
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.

5 files reviewed, 9 comments

Edit Code Review Bot Settings | Greptile

@momchil-flex momchil-flex force-pushed the momchil/mode_angle_symmetry branch from 34938d1 to 1f5ce51 Compare August 12, 2025 12:25
@momchil-flex momchil-flex changed the title Proper handling of fully anisotropic mode solver Updates for fully anisotropic mode solver Aug 12, 2025
direction,
dls,
Nxy=None,
dmin_pmc=None,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This diff is mostly because of a bit of reorganizing, but nothing substantial is changing in this file except for passing dls, Nxy and dmin_pmc around.

@momchil-flex momchil-flex force-pushed the momchil/mode_angle_symmetry branch from 0b6e9aa to ced91bb Compare August 14, 2025 07:41
Copy link
Contributor

@caseyflex caseyflex left a comment

Choose a reason for hiding this comment

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

This looks good, a couple of comments:

  • Maybe a changelog entry, at least for the change to make_eps_data
  • We can go ahead and merge this, right (with the backend after those changes)? Then when tidy3d-extras is released, we can add requires_tidy3d_extras decorators

@momchil-flex momchil-flex marked this pull request as ready for review August 29, 2025 08:16
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, no comments

Edit Code Review Bot Settings | Greptile

@momchil-flex
Copy link
Collaborator Author

Ok, yeah, we can get this in already (after a changelog item), but I'll run backend tests with this change to make sure nothing breaks when it's standalone, without the backend PR.

@momchil-flex momchil-flex force-pushed the momchil/mode_angle_symmetry branch from ced91bb to dcbe03f Compare August 29, 2025 11:52
Copy link
Contributor

Diff Coverage

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

  • tidy3d/components/mode/mode_solver.py (100%)
  • tidy3d/components/mode/solver.py (100%)
  • tidy3d/components/simulation.py (100%)

Summary

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

Looser tolerance for plane/simulation center match in mode solver symmetry
@momchil-flex momchil-flex force-pushed the momchil/mode_angle_symmetry branch from dcbe03f to 056cf28 Compare September 10, 2025 15:17
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