Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `tidy3d.plugins.design.DesignSpace.run(..., fn_post=...)` now accepts a `priority` keyword to propagate vGPU queue priority to all automatically batched simulations.
- Introduced `BroadbandPulse` for exciting simulations across a wide frequency spectrum.
- Added `interp_spec` in `ModeSpec` to allow downsampling and interpolation of waveguide modes in frequency.
- Added `interp_spec` in `EMEModeSpec` to enable faster multi-frequency EME simulations.

### Breaking Changes
- Edge singularity correction at PEC and lossy metal edges defaults to `True`.
Expand All @@ -62,6 +63,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Simulation data of batch jobs are now automatically downloaded upon their individual completion in `Batch.run()`, avoiding waiting for the entire batch to reach completion.
- Port names in `ModalComponentModeler` and `TerminalComponentModeler` can no longer include the `@` symbol.
- Improved speed of convolutions for large inputs.
- Default value of `EMEModeSpec.interp_spec` is `ModeInterpSpec.cheb(num_points=3, reduce_data=True)` for faster multi-frequency EME simulations.

### Fixed
- Ensured the legacy `Env` proxy mirrors `config.web` profile switches and preserves API URL.
Expand Down
4 changes: 3 additions & 1 deletion tests/test_components/test_eme.py
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,9 @@ def _get_mode_solver_data(modes_out=False, num_modes=3):
size=(td.inf, td.inf, 0),
center=(0, 0, offset),
freqs=[td.C_0],
mode_spec=td.ModeSpec(num_modes=num_modes),
mode_spec=td.ModeSpec(
num_modes=num_modes, interp_spec=td.ModeInterpSpec.cheb(num_points=3, reduce_data=True)
),
name=name,
)
eme_mode_data = _get_eme_mode_solver_data()
Expand Down
4 changes: 4 additions & 0 deletions tidy3d/components/data/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ def _interp_dataarray_in_freq(
DataArray
Interpolated data array with the same structure but new frequency points.
"""
# if dataarray is already stored at the correct frequencies, do nothing
if np.array_equal(freqs, data.f):
return data

# Map 'poly' to xarray's 'barycentric' method
xr_method = "barycentric" if method == "poly" else method

Expand Down
28 changes: 26 additions & 2 deletions tidy3d/components/eme/data/sim_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,27 @@ def smatrix_in_basis(
modes1 = port_modes1
if not modes2_provided:
modes2 = port_modes2
f1 = list(modes1.field_components.values())[0].f.values
f2 = list(modes2.field_components.values())[0].f.values
f1 = list(modes1.monitor.freqs)
f2 = list(modes2.monitor.freqs)

f = np.array(sorted(set(f1).intersection(f2).intersection(self.simulation.freqs)))

interp_spec1 = (
modes1.monitor.mode_spec.interp_spec if isinstance(modes1, ModeData) else None
)
interp_spec2 = (
modes2.monitor.mode_spec.interp_spec if isinstance(modes2, ModeData) else None
)

interp_overlaps = False
if interp_spec1 is not None and interp_spec2 is not None and interp_spec1 == interp_spec2:
interp_overlaps = True
else:
if interp_spec1 is not None:
modes1 = modes1.interpolated_copy
if interp_spec2 is not None:
modes2 = modes2.interpolated_copy

modes_in_1 = "mode_index" in list(modes1.field_components.values())[0].coords
modes_in_2 = "mode_index" in list(modes2.field_components.values())[0].coords

Expand Down Expand Up @@ -259,6 +275,10 @@ def smatrix_in_basis(
overlaps1 = modes1.outer_dot(port_modes1, conjugate=False)
if not modes_in_1:
overlaps1 = overlaps1.expand_dims(dim={"mode_index_0": mode_index_1}, axis=1)
if interp_overlaps:
overlaps1 = modes1._interp_dataarray_in_freq(
overlaps1, freqs=f, method=interp_spec1.method
)
O1 = overlaps1.sel(f=f, mode_index_1=keep_mode_inds1)

O1out = O1.rename(mode_index_0="mode_index_out", mode_index_1="mode_index_out_old")
Expand Down Expand Up @@ -288,6 +308,10 @@ def smatrix_in_basis(
overlaps2 = modes2.outer_dot(port_modes2, conjugate=False)
if not modes_in_2:
overlaps2 = overlaps2.expand_dims(dim={"mode_index_0": mode_index_2}, axis=1)
if interp_overlaps:
overlaps2 = modes2._interp_dataarray_in_freq(
overlaps2, freqs=f, method=interp_spec2.method
)
O2 = overlaps2.sel(f=f, mode_index_1=keep_mode_inds2)

O2out = O2.rename(mode_index_0="mode_index_out", mode_index_1="mode_index_out_old")
Expand Down
19 changes: 10 additions & 9 deletions tidy3d/components/eme/grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
from tidy3d.components.base import Tidy3dBaseModel, skip_if_fields_missing
from tidy3d.components.geometry.base import Box
from tidy3d.components.grid.grid import Coords1D
from tidy3d.components.mode_spec import ModeSpec
from tidy3d.components.mode_spec import ModeInterpSpec, ModeSpec
from tidy3d.components.structure import Structure
from tidy3d.components.types import ArrayFloat1D, Axis, Coordinate, Size, TrackFreq
from tidy3d.components.types import ArrayFloat1D, Axis, Coordinate, Size
from tidy3d.constants import RADIAN, fp_eps, inf
from tidy3d.exceptions import SetupError, ValidationError

Expand All @@ -26,13 +26,14 @@
class EMEModeSpec(ModeSpec):
"""Mode spec for EME cells. Overrides some of the defaults and allowed values."""

track_freq: Union[TrackFreq, None] = pd.Field(
None,
title="Mode Tracking Frequency",
description="Parameter that turns on/off mode tracking based on their similarity. "
"Can take values ``'lowest'``, ``'central'``, or ``'highest'``, which correspond to "
"mode tracking based on the lowest, central, or highest frequency. "
"If ``None`` no mode tracking is performed, which is the default for best performance.",
interp_spec: Optional[ModeInterpSpec] = pd.Field(
ModeInterpSpec.cheb(num_points=3, reduce_data=True),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@momchil-flex We could change this to be GVD based (i.e. polynomial with three sampling points concentrated around the central frequency). But I'm not sure that's better in general. It can lower the accuracy towards the edge of the spectrum, and I haven't had a problem with mode tracking. And the user can customize this if needed

title="Mode frequency interpolation specification",
description="Specification for computing modes at a reduced set of frequencies and "
"interpolating to obtain results at all requested frequencies. This can significantly "
"reduce computational cost for broadband simulations where modes vary smoothly with "
"frequency. Requires frequency tracking to be enabled (``sort_spec.track_freq`` must "
"not be ``None``) to ensure consistent mode ordering across frequencies.",
)
Comment on lines +29 to 37
Copy link

Choose a reason for hiding this comment

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

logic: The description states "Requires frequency tracking to be enabled (sort_spec.track_freq must not be None)", but EMEModeSpec no longer has a track_freq field (it was removed in this PR). The docstring should be updated to reflect the actual requirements. Consider checking the parent ModeSpec class's sort_spec field instead if tracking is needed.

Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/components/eme/grid.py
Line: 29:37

Comment:
**logic:** The description states "Requires frequency tracking to be enabled (`sort_spec.track_freq` must not be `None`)", but `EMEModeSpec` no longer has a `track_freq` field (it was removed in this PR). The docstring should be updated to reflect the actual requirements. Consider checking the parent `ModeSpec` class's `sort_spec` field instead if tracking is needed.

How can I resolve this? If you propose a fix, please make it concise.


angle_theta: Literal[0.0] = pd.Field(
Expand Down
12 changes: 12 additions & 0 deletions tidy3d/components/eme/simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,18 @@ def _monitor_freqs(self, monitor: Monitor) -> list[pd.NonNegativeFloat]:
return list(self.freqs)
return list(monitor.freqs)

def _monitor_mode_freqs(self, monitor: EMEModeSolverMonitor) -> list[pd.NonNegativeFloat]:
"""Monitor frequencies."""
freqs = set()
cell_inds = self._monitor_eme_cell_indices(monitor=monitor)
for cell_ind in cell_inds:
interp_spec = self.eme_grid.mode_specs[cell_ind].interp_spec
if interp_spec is None:
freqs |= set(self.freqs)
else:
freqs |= set(interp_spec.sampling_points(self.freqs))
return list(freqs)

def _monitor_num_freqs(self, monitor: Monitor) -> int:
"""Total number of freqs included in monitor."""
return len(self._monitor_freqs(monitor=monitor))
Expand Down
4 changes: 2 additions & 2 deletions tidy3d/components/mode/mode_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
from tidy3d.components.types.mode_spec import ModeSpecType
from tidy3d.components.types.monitor_data import ModeSolverDataType
from tidy3d.components.validators import (
_warn_interp_num_points,

Check failure on line 78 in tidy3d/components/mode/mode_solver.py

View workflow job for this annotation

GitHub Actions / verify-linting

Ruff (F401)

tidy3d/components/mode/mode_solver.py:78:5: F401 `tidy3d.components.validators._warn_interp_num_points` imported but unused
validate_freqs_min,
validate_freqs_not_empty,
)
Expand Down Expand Up @@ -515,8 +515,8 @@
A mode solver data type object containing the effective index and mode fields.
"""

if self.mode_spec.interp_spec is not None:
_warn_interp_num_points(self.mode_spec.interp_spec, self.freqs)
# if self.mode_spec.interp_spec is not None:
# _warn_interp_num_points(self.mode_spec.interp_spec, self.freqs)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dbochkov-flexcompute Do we need these warnings? They are annoying in an EME simulation where interpolation is the default, and falling back to the actual frequencies specified is even better (not worth warning about necessarily)

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I think I was overcautious here. I think it was making more sense when interp_spec was outside of mode_spec, because to enable that the user had to make a conscious effort of specifying interp_spec in a monitor and it felt like we should let them know it was for nothing lol

Comment on lines +518 to +519
Copy link

Choose a reason for hiding this comment

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

style: validation commented out. Add a comment explaining why this warning was disabled.

Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/components/mode/mode_solver.py
Line: 518:519

Comment:
**style:** validation commented out. Add a comment explaining why this warning was disabled.

How can I resolve this? If you propose a fix, please make it concise.


if self.mode_spec.angle_rotation and np.abs(self.mode_spec.angle_theta) > 0:
return self.rotated_mode_solver_data
Expand Down
2 changes: 1 addition & 1 deletion tidy3d/components/mode_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ def sampling_points(self, freqs: FreqArray) -> FreqArray:
>>> interp_spec = ModeInterpSpec.cheb(num_points=10)
>>> sampling_freqs = interp_spec.sampling_points(freqs)
"""
if self.num_points > len(freqs):
if self.num_points >= len(freqs):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dbochkov-flexcompute not really needed for EME, but should we make this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, that's more accurate, thanks for catching that

return freqs
Comment on lines +436 to 437
Copy link

Choose a reason for hiding this comment

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

style: changed from > to >= but now the validation warning is commented out in monitor.py:440 and mode_solver.py:518-519. Users won't be warned when num_points >= len(freqs), which defeats the purpose of this optimization. Either restore the warning or add a comment explaining why it's acceptable.

Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/components/mode_spec.py
Line: 436:437

Comment:
**style:** changed from `>` to `>=` but now the validation warning is commented out in monitor.py:440 and mode_solver.py:518-519. Users won't be warned when `num_points >= len(freqs)`, which defeats the purpose of this optimization. Either restore the warning or add a comment explaining why it's acceptable.

How can I resolve this? If you propose a fix, please make it concise.

return self.sampling_spec.sampling_points(freqs)

Expand Down
11 changes: 9 additions & 2 deletions tidy3d/components/monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
assert_plane,
validate_freqs_min,
validate_freqs_not_empty,
validate_interp_num_points,

Check failure on line 40 in tidy3d/components/monitor.py

View workflow job for this annotation

GitHub Actions / verify-linting

Ruff (F401)

tidy3d/components/monitor.py:40:5: F401 `.validators.validate_interp_num_points` imported but unused
)
from .viz import ARROW_ALPHA, ARROW_COLOR_MONITOR

Expand Down Expand Up @@ -429,15 +429,22 @@
)
return val

@property
def _stored_freqs(self) -> list[float]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dbochkov-flexcompute this is to update _storage_size_solver for ModeSolverMonitor (although the full freqs will still be used for ModeMonitor)

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, yeah, I updated ModeMonitor and ModeSolverMonitor directly but forgot about AbstractModeMonitor

"""Return actually stored frequencies of the data."""
return self.mode_spec._sampling_freqs_mode_solver_data(freqs=self.freqs)

def _storage_size_solver(self, num_cells: int, tmesh: ArrayFloat1D) -> int:
"""Size of intermediate data recorded by the monitor during a solver run."""
# Need to store all fields on the mode surface
bytes_single = BYTES_COMPLEX * num_cells * len(self.freqs) * self.mode_spec.num_modes * 6
bytes_single = (
BYTES_COMPLEX * num_cells * len(self._stored_freqs) * self.mode_spec.num_modes * 6
)
if self.mode_spec.precision == "double":
return 2 * bytes_single
return bytes_single

_warn_interp_num_points = validate_interp_num_points()
# _warn_interp_num_points = validate_interp_num_points()
Copy link

Choose a reason for hiding this comment

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

style: validation commented out instead of removed. Add a comment explaining why (e.g., "Warning disabled because EME now defaults to interpolation" or similar context).

Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/components/monitor.py
Line: 440:440

Comment:
**style:** validation commented out instead of removed. Add a comment explaining why (e.g., "Warning disabled because EME now defaults to interpolation" or similar context).

How can I resolve this? If you propose a fix, please make it concise.



class FieldMonitor(AbstractFieldMonitor, FreqMonitor):
Expand Down
Loading