Skip to content

Conversation

@momchil-flex
Copy link
Collaborator

@momchil-flex momchil-flex commented Oct 16, 2025

Greptile Overview

Updated On: 2025-10-16 09:46:16 UTC

Greptile Summary

This PR adds Gaussian overlap monitor functionality to the Tidy3D simulation system through FXC-3275. The implementation introduces two new monitor classes: GaussianOverlapMonitor for circular Gaussian beam overlap calculations and AstigmaticGaussianOverlapMonitor for elliptical beams with different waist parameters in each direction.

The change refactors the monitor class hierarchy by creating a new AbstractOverlapMonitor base class that contains shared functionality between existing mode monitors and the new Gaussian monitors. This includes fields for store_fields_direction, colocate, and conjugated_dot_product, which were moved from AbstractModeMonitor to enable code reuse. The Gaussian monitors add specialized fields for beam parameters including angles, polarization, and waist characteristics.

The implementation follows established patterns in the codebase with proper inheritance, documentation, and type safety. Test coverage is included for both monitor classes with basic functionality verification and integration into the comprehensive monitor test suite.

Important Files Changed

Changed Files
Filename Score Overview
tidy3d/components/monitor.py 4/5 Major refactoring adding AbstractOverlapMonitor base class and two new Gaussian overlap monitor implementations
tidy3d/components/types/monitor.py 5/5 Simple type additions registering the new monitor classes in the type system
tests/test_components/test_monitor.py 5/5 Basic test coverage for the new monitor classes with integration into existing test suite

Confidence score: 4/5

  • This PR is safe to merge with good architectural design and proper testing
  • Score reflects solid implementation following established patterns, but monitor hierarchy refactoring in critical simulation code requires careful validation
  • Pay close attention to the monitor.py file to ensure the refactoring doesn't break existing mode monitor functionality

Sequence Diagram

sequenceDiagram
    participant User
    participant Simulation as "Simulation"
    participant GaussianMonitor as "GaussianOverlapMonitor"
    participant AstigmaticGaussianMonitor as "AstigmaticGaussianOverlapMonitor"
    participant AbstractGaussianOverlapMonitor as "AbstractGaussianOverlapMonitor"
    participant FreqMonitor as "FreqMonitor"
    participant Solver as "FDTD Solver"

    User->>GaussianMonitor: "Create GaussianOverlapMonitor(size, freqs, waist_radius, waist_distance)"
    GaussianMonitor->>AbstractGaussianOverlapMonitor: "Inherit base functionality"
    AbstractGaussianOverlapMonitor->>FreqMonitor: "Inherit frequency domain processing"
    
    User->>AstigmaticGaussianMonitor: "Create AstigmaticGaussianOverlapMonitor(size, freqs, waist_sizes, waist_distances)"
    AstigmaticGaussianMonitor->>AbstractGaussianOverlapMonitor: "Inherit base functionality"
    
    User->>Simulation: "Add monitors to simulation"
    Simulation->>GaussianMonitor: "Validate monitor configuration"
    Simulation->>AstigmaticGaussianMonitor: "Validate monitor configuration"
    
    User->>Simulation: "Run simulation"
    Simulation->>Solver: "Initialize FDTD solver"
    Solver->>GaussianMonitor: "Record field data at frequencies"
    Solver->>AstigmaticGaussianMonitor: "Record field data at frequencies"
    
    GaussianMonitor->>AbstractGaussianOverlapMonitor: "Compute Gaussian beam overlap"
    AstigmaticGaussianMonitor->>AbstractGaussianOverlapMonitor: "Compute astigmatic Gaussian beam overlap"
    
    AbstractGaussianOverlapMonitor->>Solver: "Return complex amplitudes for +/- directions"
    Solver->>Simulation: "Store monitor data"
    Simulation->>User: "Return simulation results with Gaussian overlap data"
Loading

Note

Introduces Gaussian-beam field decomposition and integrates it into port-based S‑matrix flows.

  • Adds GaussianOverlapMonitor and AstigmaticGaussianOverlapMonitor to compute overlaps onto (astigmatic) Gaussian beams; exports, schemas, and types updated
  • Refactors monitor hierarchy with new AbstractOverlapMonitor shared by mode and Gaussian overlap monitors; moves shared fields (store_fields_direction, colocate, conjugated_dot_product)
  • Adds FieldOverlapData for overlap amplitudes and normalization; updates data utils and tests
  • Adds GaussianPort and AstigmaticGaussianPort; ModalComponentModeler now accepts modal or Gaussian ports, constructs appropriate sources/monitors, and builds S‑matrices from overlap amps
  • Updates beam/gaussian waist semantics to be direction‑independent (backward propagation behavior changed) and adjusts warnings; monitor/source docs clarified
  • Test suite extended for new monitors/ports and S‑matrix paths; schemas (Simulation.json, TerminalComponentModeler.json) include new monitor types

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

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

@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch from 61d5146 to 61cebca Compare October 16, 2025 11:34
@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025

Diff Coverage

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

  • tidy3d/components/beam.py (75.0%): Missing lines 395
  • tidy3d/components/data/monitor_data.py (86.2%): Missing lines 1748,1762,1776,1791
  • tidy3d/components/monitor.py (83.0%): Missing lines 381-382,384,396,410,414-415,563
  • tidy3d/plugins/smatrix/init.py (100%)
  • tidy3d/plugins/smatrix/analysis/modal.py (100%)
  • tidy3d/plugins/smatrix/component_modelers/modal.py (97.7%): Missing lines 229
  • tidy3d/plugins/smatrix/ports/modal.py (93.0%): Missing lines 102,189,251

Summary

  • Total: 170 lines
  • Missing: 17 lines
  • Coverage: 90%

tidy3d/components/beam.py

Lines 391-399

  391         """
  392 
  393         w_0, z_0 = self.waist_radius, self.waist_distance
  394         if self.direction == "-":
! 395             z_0 = -z_0
  396         z_r = w_0**2 * k0 / 2  # shape k0
  397         w_z = w_0 * np.sqrt(1 + ((z + z_0) / z_r) ** 2)  # shape (Np, Nk0)
  398         # inv_r_z shape (Np, Nk0)
  399         inv_r_z = (z + z_0) / ((z + z_0) ** 2 + z_r**2)

tidy3d/components/data/monitor_data.py

Lines 1744-1752

  1744 
  1745     def normalize(self, source_spectrum_fn) -> AbstractOverlapData:
  1746         """Return copy of self after normalization is applied using source spectrum function."""
  1747         if self.amps is None:
! 1748             return self.copy()
  1749         source_freq_amps = source_spectrum_fn(self.amps.f)[None, :, None]
  1750         new_amps = (self.amps / source_freq_amps).astype(self.amps.dtype)
  1751         return self.copy(update={"amps": new_amps})

Lines 1758-1766

  1758         """
  1759         mnt = self.monitor
  1760 
  1761         if not mnt.store_fields_direction:
! 1762             return self.copy()
  1763 
  1764         # Time reversal
  1765         new_data = {}
  1766         for comp, field in self.field_components.items():

Lines 1772-1780

  1772         # switch direction in the monitor
  1773         new_dir = "+" if mnt.store_fields_direction == "-" else "-"
  1774         update_dict = {"store_fields_direction": new_dir}
  1775         if hasattr(mnt, "direction"):
! 1776             update_dict["direction"] = new_dir
  1777         new_data["monitor"] = mnt.updated_copy(**update_dict)
  1778         return self.copy(update=new_data)
  1779 

Lines 1787-1795

  1787         self, dataset_names: list[str], fwidth: float
  1788     ) -> list[Union[CustomCurrentSource, PointDipole]]:
  1789         """Converts a :class:`.FieldData` to a list of adjoint current or point sources."""
  1790 
! 1791         raise NotImplementedError("Could not formulate adjoint source for overlap monitor output.")
  1792 
  1793 
  1794 class ModeData(ModeSolverDataset, AbstractOverlapData):
  1795     """

tidy3d/components/monitor.py

Lines 377-388

  377 
  378         if not self._draw_overlap_arrows:
  379             return ax
  380 
! 381         kwargs_alpha = patch_kwargs.get("alpha")
! 382         arrow_alpha = ARROW_ALPHA if kwargs_alpha is None else kwargs_alpha
  383 
! 384         ax = self._plot_arrow(
  385             x=x,
  386             y=y,
  387             z=z,
  388             ax=ax,

Lines 392-400

  392             color=ARROW_COLOR_MONITOR,
  393             alpha=arrow_alpha,
  394             both_dirs=True,
  395         )
! 396         return ax
  397 
  398     @cached_property
  399     def _dir_arrow(self) -> tuple[float, float, float]:
  400         """Monitor direction normal vector in cartesian coordinates."""

Lines 406-419

  406 
  407     @property
  408     def _angles(self) -> tuple[float, float]:
  409         """Angle tuple (theta, phi) in radians. Children override to supply values."""
! 410         return (0.0, 0.0)
  411 
  412     def _storage_size_solver(self, num_cells: int, tmesh: ArrayFloat1D) -> int:
  413         """Default size of intermediate data; store all fields on plane for overlap."""
! 414         num_sample = len(getattr(self, "freqs", [0]))
! 415         return BYTES_COMPLEX * num_cells * num_sample * 6
  416 
  417 
  418 class AbstractModeMonitor(AbstractOverlapMonitor):
  419     """:class:`Monitor` that records mode-related data."""

Lines 559-567

  559     )
  560 
  561     @property
  562     def _angles(self) -> tuple[float, float]:
! 563         return (self.angle_theta, self.angle_phi)
  564 
  565     def storage_size(self, num_cells: int, tmesh: ArrayFloat1D) -> int:
  566         """Size of monitor storage given the number of points after discretization."""
  567         # store complex amplitudes for +/- directions

tidy3d/plugins/smatrix/component_modelers/modal.py

Lines 225-233

  225             theta = port.angle_theta
  226             phi = port.angle_phi
  227             cos_theta = np.cos(theta)
  228             if np.abs(cos_theta) < GLANCING_CUTOFF:
! 229                 raise SetupError(
  230                     "Cannot shift Gaussian port at glancing incidence. "
  231                     "Adjust angle_theta or use a different injection axis."
  232                 )
  233             tan_theta = np.sin(theta) / cos_theta

tidy3d/plugins/smatrix/ports/modal.py

Lines 98-106

   98         **kwargs: Any,
   99     ) -> ModeSource:
  100         """Create a ModeSource matching this modal port."""
  101         if source_time is None:
! 102             source_time = GaussianPulse(freq0=freq0, fwidth=fwidth)
  103         return ModeSource(
  104             center=self.center,
  105             size=self.size,
  106             source_time=source_time,

Lines 185-193

  185         **kwargs: Any,
  186     ) -> GaussianBeam:
  187         """Create a GaussianBeam matching this gaussian port. Ignores mode_index."""
  188         if source_time is None:
! 189             source_time = GaussianPulse(freq0=freq0, fwidth=fwidth)
  190         return GaussianBeam(
  191             center=self.center,
  192             size=self.size,
  193             source_time=source_time,

Lines 247-255

  247         **kwargs: Any,
  248     ) -> AstigmaticGaussianBeam:
  249         """Create an AstigmaticGaussianBeam matching this port. Ignores mode_index."""
  250         if source_time is None:
! 251             source_time = GaussianPulse(freq0=freq0, fwidth=fwidth)
  252         return AstigmaticGaussianBeam(
  253             center=self.center,
  254             size=self.size,
  255             source_time=source_time,

@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch 2 times, most recently from daddcfd to a61be33 Compare October 17, 2025 12:01
Copilot AI review requested due to automatic review settings December 2, 2025 12:36
@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch from 9403b10 to e2b60b2 Compare December 2, 2025 12:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Gaussian beam overlap monitor functionality to support S-matrix calculations with Gaussian ports. The implementation introduces two new monitor classes (GaussianOverlapMonitor and AstigmaticGaussianOverlapMonitor) and corresponding port classes, along with a refactoring of the monitor hierarchy to share common overlap functionality.

  • Refactored monitor hierarchy by creating AbstractOverlapMonitor base class to share code between mode and Gaussian overlap monitors
  • Added GaussianOverlapMonitor and AstigmaticGaussianOverlapMonitor for computing field overlap with Gaussian beams
  • Introduced GaussianPort and AstigmaticGaussianPort to enable S-matrix calculations using Gaussian beams
  • Modified beam profile waist distance handling based on propagation direction (breaking change)

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
tidy3d/components/monitor.py Adds AbstractOverlapMonitor base class and two new Gaussian overlap monitor implementations with field refactoring
tidy3d/components/data/monitor_data.py Introduces AbstractOverlapData and FieldOverlapData classes for Gaussian overlap monitor data
tidy3d/components/beam.py Updates beam profile waist distance sign handling based on direction (breaking change)
tidy3d/plugins/smatrix/ports/modal.py Adds AbstractPort hierarchy and Gaussian port implementations with to_monitor/to_source methods
tidy3d/plugins/smatrix/component_modelers/modal.py Refactors to support both modal and Gaussian ports in S-matrix calculations
tidy3d/plugins/smatrix/analysis/modal.py Updates S-matrix construction to handle both modal and Gaussian overlap data
tidy3d/components/types/monitor.py Registers new monitor types in the type system
tidy3d/components/types/monitor_data.py Adds FieldOverlapData to monitor data type system
tidy3d/plugins/smatrix/init.py Exports new Gaussian port classes
tidy3d/init.py Exports new monitor and data classes to public API
tests/utils.py Adds test data generation functions for Gaussian overlap monitors
tests/test_components/test_monitor.py Adds basic tests for new Gaussian overlap monitors
tests/test_data/test_monitor_data.py Adds tests for FieldOverlapData
tests/test_plugins/smatrix/test_component_modeler.py Updates tests to include Gaussian ports in S-matrix calculations
schemas/*.json Updates JSON schemas with new monitor class definitions
Comments suppressed due to low confidence (1)

tidy3d/components/beam.py:210

  • This comment appears to contain commented-out code.
            # if field == "H":
            #     field_vals *= -1

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch from 198fed3 to 609e450 Compare January 16, 2026 12:40
@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch from 609e450 to 5b21861 Compare January 16, 2026 14:38
@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch from 5b21861 to 6805519 Compare January 16, 2026 14:58
@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch from 6805519 to 5e2c5ef Compare January 16, 2026 15:17
@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch 2 times, most recently from 7b7700b to eabd858 Compare January 27, 2026 19:31
@momchil-flex momchil-flex added this pull request to the merge queue Jan 28, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 28, 2026
@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch from eabd858 to 428e001 Compare January 28, 2026 09:30
@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch from 428e001 to f438446 Compare January 28, 2026 10:07
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.

@momchil-flex momchil-flex force-pushed the momchil/gaussian_monitor branch from f438446 to 3bb6f49 Compare January 28, 2026 10:31
@momchil-flex momchil-flex added this pull request to the merge queue Jan 28, 2026
Merged via the queue into develop with commit c3a04e1 Jan 28, 2026
33 of 37 checks passed
@momchil-flex momchil-flex deleted the momchil/gaussian_monitor branch January 28, 2026 12:02
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