-
Notifications
You must be signed in to change notification settings - Fork 58
add a TerminalSpec for RF specific mode information #2444
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.
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
base: develop
Are you sure you want to change the base?
Conversation
Still very rough, and I am not particularly happy about needing to define all of these different path spec types. But right now it is the best way I found to get around the circular dependency issue I am having. Basically, I cannot import path integral code into @weiliangjin2021 @dbochkov-flexcompute Any ideas to solve this? Otherwise you can retrieve impedance info by doing something as simple as:
|
Nah, having path specs seems to be the best. I'll just add some factory class to convert path specs to the correct path integral version. |
44634db
to
e12f1a7
Compare
Ready for review finally! The main feature this PR provided is automatically computing impedance and adding it to the
to some object that calculates and returns mode data. The simulation will not be affected if used in a Now, to get around all of the circular dependence stuff, I had to split the path integral classes in the microwave plugin into the specification part, and the part that actually computes the integral. At some later point we will most likely move the everything out of the microwave plugin, since these path integrals are needed for many things in RF. If anyone has suggestions on how to better get around these issues, please let me know! The main issue was the need to import the monitor data module in the path integral modules. Finally, the auto generated paths are bounding boxes around each conductor, which will work for 99% of transmission lines, but there might be improvements needed in the future to auto generate the line string type path integrals for handling more complex structures. |
@yuanshen-flexcompute It would be nice to test out this automatic version of impedance calculation on some of your test cases to make sure I did not miss anything. |
Code Coverage Summary
Diff against develop
Results for commit: c5f03f3 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Changed Files Coverage
Results for commit: c5f03f3 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
3781b06
to
c5f03f3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Gave it a first pass, didn't spot anything major, just some minor comments/questions so far
3f3195e
to
39318d6
Compare
Diff CoverageDiff: origin/develop...HEAD, staged and unstaged changes
Summary
tidy3d/components/data/monitor_data.pyLines 2063-2072 2063 info["wg TE fraction"] = self.pol_fraction_waveguide["te"]
2064 info["wg TM fraction"] = self.pol_fraction_waveguide["tm"]
2065
2066 if self.Z0 is not None:
! 2067 info["Re(Z0)"] = self.Z0.real
! 2068 info["Im(Z0)"] = self.Z0.imag
2069
2070 return xr.Dataset(data_vars=info)
2071
2072 def to_dataframe(self) -> DataFrame: tidy3d/components/geometry/utils.pyLines 71-90 71 Flat list of non-empty geometries matching the specified types.
72 """
73 # Handle single Shapely object by wrapping it in a list
74 if isinstance(geoms, Shapely):
! 75 geoms = [geoms]
76
77 flat = []
78 for geom in geoms:
79 if geom.is_empty:
! 80 continue
81 if isinstance(geom, keep_types):
82 flat.append(geom)
! 83 elif isinstance(geom, (MultiPolygon, MultiLineString, MultiPoint, GeometryCollection)):
! 84 flat.extend(flatten_shapely_geometries(geom.geoms, keep_types))
! 85 elif isinstance(geom, BaseGeometry) and hasattr(geom, "geoms"):
! 86 flat.extend(flatten_shapely_geometries(geom.geoms, keep_types))
87 return flat
88
89
90 def merging_geometries_on_plane( Lines 509-517 509 else: # SnapType.Contract
510 min_upper_bound_idx += snap_margin
511 max_upper_bound_idx -= snap_margin
512 if max_upper_bound_idx < min_upper_bound_idx:
! 513 raise SetupError("The supplied 'snap_buffer' is too large for this contraction.")
514 min_snap = get_upper_bound(interval_min, coords, min_upper_bound_idx, rel_tol=rtol)
515 max_snap = get_lower_bound(interval_max, coords, max_upper_bound_idx, rel_tol=rtol)
516 return (min_snap, max_snap) tidy3d/components/microwave/microwave_mode_spec.pyLines 49-57 49 @property
50 def num_voltage_specs(self) -> Optional[int]:
51 """The number of voltage specifications supplied."""
52 if type(self.voltage_spec) is tuple:
! 53 return len(self.voltage_spec)
54 return None
55
56 @property
57 def num_current_specs(self) -> Optional[int]: Lines 56-64 56 @property
57 def num_current_specs(self) -> Optional[int]:
58 """The number of current specifications supplied."""
59 if type(self.current_spec) is tuple:
! 60 return len(self.current_spec)
61 return None
62
63 @pd.validator("current_spec", always=True)
64 def check_path_spec_combinations(cls, val, values): tidy3d/components/microwave/path_integral_factory.pyLines 48-56 48 v_integral = VoltageIntegralAxisAligned(**path_spec.dict(exclude={"type"}))
49 elif isinstance(path_spec, CustomVoltageIntegral2DSpec):
50 v_integral = CustomVoltageIntegral2D(**path_spec.dict(exclude={"type"}))
51 else:
! 52 raise ValidationError(f"Unsupported voltage path specification type: {type(path_spec)}")
53 return v_integral
54
55
56 def make_current_integral(path_spec: CurrentPathSpecTypes) -> CurrentIntegralTypes: Lines 74-82 74 i_integral = CustomCurrentIntegral2D(**path_spec.dict(exclude={"type"}))
75 elif isinstance(path_spec, CompositeCurrentIntegralSpec):
76 i_integral = CompositeCurrentIntegral(**path_spec.dict(exclude={"type"}))
77 else:
! 78 raise ValidationError(f"Unsupported current path specification type: {type(path_spec)}")
79 return i_integral
80
81
82 def make_path_integrals( Lines 117-129 117 sim.symmetry,
118 sim.bounding_box,
119 monitor.colocate,
120 )
! 121 i_specs = (i_spec,) * monitor.mode_spec.num_modes
! 122 if v_specs is None:
! 123 v_specs = (None,) * monitor.mode_spec.num_modes
! 124 if i_specs is None:
! 125 i_specs = (None,) * monitor.mode_spec.num_modes
126
127 except ValidationError as e:
128 raise SetupError(
129 f"Failed to auto-generate path specification for impedance calculation in monitor '{monitor.name}'." Lines 128-149 128 raise SetupError(
129 f"Failed to auto-generate path specification for impedance calculation in monitor '{monitor.name}'."
130 ) from e
131
! 132 try:
! 133 path_integrals = []
! 134 for v_spec, i_spec in zip(v_specs, i_specs):
! 135 v_integral = None
! 136 i_integral = None
! 137 if v_spec is not None:
! 138 v_integral = make_voltage_integral(v_spec)
! 139 if i_spec is not None:
! 140 i_integral = make_current_integral(i_spec)
! 141 path_integrals.append((v_integral, i_integral))
! 142 path_integrals = tuple(path_integrals)
! 143 except Exception as e:
! 144 raise SetupError(
145 f"Failed to construct path integrals from the microwave mode specification for monitor '{monitor.name}'. "
146 "Please create a github issue so that the problem can be investigated."
147 ) from e
! 148 return path_integrals tidy3d/components/microwave/path_spec.pyLines 103-111 103 """Axis for performing integration."""
104 for index, value in enumerate(self.size):
105 if value != 0:
106 return index
! 107 raise Tidy3dError("Failed to identify axis.")
108
109 def _vertices_2D(self, axis: Axis) -> tuple[Coordinate2D, Coordinate2D]:
110 """Returns the two vertices of this path in the plane defined by ``axis``."""
111 min = self.bounds[0] Lines 161-183 161 -------
162 VoltageIntegralAxisAlignedSpec
163 The created path integral for computing voltage between the two terminals.
164 """
! 165 axis_positions = Geometry.parse_two_xyz_kwargs(x=x, y=y, z=z)
166 # Calculate center and size of the future box
! 167 midpoint = (plus_terminal + minus_terminal) / 2
! 168 length = np.abs(plus_terminal - minus_terminal)
! 169 center = [midpoint, midpoint, midpoint]
! 170 size = [length, length, length]
! 171 for axis, position in axis_positions:
! 172 size[axis] = 0
! 173 center[axis] = position
174
! 175 direction = "+"
! 176 if plus_terminal < minus_terminal:
! 177 direction = "-"
178
! 179 return VoltageIntegralAxisAlignedSpec(
180 center=center,
181 size=size,
182 extrapolate_to_endpoints=extrapolate_to_endpoints,
183 snap_path_to_grid=snap_path_to_grid, Lines 265-273 265 """Axis normal to loop"""
266 for index, value in enumerate(self.size):
267 if value == 0:
268 return index
! 269 raise Tidy3dError("Failed to identify axis.")
270
271 def _to_path_integral_specs(
272 self, h_horizontal=None, h_vertical=None
273 ) -> tuple[AxisAlignedPathIntegralSpec, ...]: Lines 464-478 464
465 @staticmethod
466 def _compute_dl_component(coord_array: xr.DataArray, closed_contour=False) -> np.array:
467 """Computes the differential length element along the integration path."""
! 468 dl = np.gradient(coord_array)
! 469 if closed_contour:
470 # If the contour is closed, we can use central difference on the starting/end point
471 # which will be more accurate than the default forward/backward choice in np.gradient
! 472 grad_end = np.gradient([coord_array[-2], coord_array[0], coord_array[1]])
! 473 dl[0] = dl[-1] = grad_end[1]
! 474 return dl
475
476 @classmethod
477 def from_circular_path(
478 cls, center: Coordinate, radius: float, num_points: int, normal_axis: Axis, clockwise: bool Lines 499-530 499 :class:`.CustomPathIntegral2DSpec`
500 A path integral defined on a circular path.
501 """
502
! 503 def generate_circle_coordinates(radius: float, num_points: int, clockwise: bool):
504 """Helper for generating x,y vertices around a circle in the local coordinate frame."""
! 505 sign = 1.0
! 506 if clockwise:
! 507 sign = -1.0
! 508 angles = np.linspace(0, sign * 2 * np.pi, num_points, endpoint=True)
! 509 xt = radius * np.cos(angles)
! 510 yt = radius * np.sin(angles)
! 511 return (xt, yt)
512
513 # Get transverse axes
! 514 normal_center, trans_center = Geometry.pop_axis(center, normal_axis)
515
516 # These x,y coordinates in the local coordinate frame
! 517 if normal_axis == 1:
518 # Handle special case when y is the axis that is popped
! 519 clockwise = not clockwise
! 520 xt, yt = generate_circle_coordinates(radius, num_points, clockwise)
! 521 xt += trans_center[0]
! 522 yt += trans_center[1]
! 523 circle_vertices = np.column_stack((xt, yt))
524 # Close the contour exactly
! 525 circle_vertices[-1, :] = circle_vertices[0, :]
! 526 return cls(axis=normal_axis, position=normal_center, vertices=circle_vertices)
527
528 @cached_property
529 def is_closed_contour(self) -> bool:
530 """Returns ``true`` when the first vertex equals the last vertex.""" Lines 562-578 562
563 @cached_property
564 def sign(self) -> Direction:
565 """Uses the ordering of the vertices to determine the direction of the current flow."""
! 566 linestr = shapely.LineString(coordinates=self.vertices)
! 567 is_ccw = shapely.is_ccw(linestr)
568 # Invert statement when the vertices are given as (x, z)
! 569 if self.axis == 1:
! 570 is_ccw = not is_ccw
! 571 if is_ccw:
! 572 return "+"
573 else:
! 574 return "-"
575
576
577 class CustomVoltageIntegral2DSpec(CustomPathIntegral2DSpec):
578 """Class for specfying the computation of voltage between two points defined by a custom path. Lines 697-705 697 linestr = shapely.LineString(coordinates=self.vertices)
698 is_ccw = shapely.is_ccw(linestr)
699 # Invert statement when the vertices are given as (x, z)
700 if self.axis == 1:
! 701 is_ccw = not is_ccw
702 if is_ccw:
703 return "+"
704 else:
705 return "-" tidy3d/components/mode/mode_solver.pyLines 501-509 501
502 mode_solver_data = self._filter_components(mode_solver_data)
503 # Calculate and add the characteristic impedance
504 if self.mode_spec.microwave_mode_spec is not None:
! 505 mode_solver_data = self._add_characteristic_impedance(mode_solver_data)
506 return mode_solver_data
507
508 @cached_property
509 def bend_axis_3d(self) -> Axis: Lines 1317-1325 1317 def _add_characteristic_impedance(self, mode_solver_data: ModeSolverData):
1318 """Calculate and add characteristic impedance to ``mode_solver_data`` with the path specifications.
1319 If they were not supplied by the user, then create a specification automatically.
1320 """
! 1321 path_integrals = make_path_integrals(
1322 self.mode_spec.microwave_mode_spec,
1323 self.to_monitor(name=MODE_MONITOR_NAME),
1324 self.simulation,
1325 ) Lines 1323-1343 1323 self.to_monitor(name=MODE_MONITOR_NAME),
1324 self.simulation,
1325 )
1326 # Need to operate on the full symmetry expanded fields
! 1327 mode_solver_data_expanded = mode_solver_data.symmetry_expanded_copy
! 1328 Z0_list = []
! 1329 for mode_index in range(self.mode_spec.num_modes):
! 1330 integral_pair = path_integrals[mode_index]
! 1331 impedance_calc = ImpedanceCalculator(
1332 voltage_integral=integral_pair[0], current_integral=integral_pair[1]
1333 )
! 1334 single_mode_data = mode_solver_data_expanded._isel(mode_index=[mode_index])
! 1335 Z0 = impedance_calc.compute_impedance(single_mode_data)
! 1336 Z0_list.append(Z0)
! 1337 all_mode_Z0 = xr.concat(Z0_list, dim="mode_index")
! 1338 all_mode_Z0 = ImpedanceCalculator._set_data_array_attributes(all_mode_Z0)
! 1339 return mode_solver_data.updated_copy(Z0=all_mode_Z0)
1340
1341 @cached_property
1342 def data(self) -> ModeSolverData:
1343 """:class:`.ModeSolverData` containing the field and effective index data. tidy3d/components/mode_spec.pyLines 260-268 260 val.num_current_specs is None or val.num_current_specs == num_modes
261 )
262
263 if not valid_number_voltage_specs:
! 264 raise SetupError(
265 f"Given {val.num_voltage_specs} voltage specifications, but the number of modes requested is {num_modes}. "
266 "Please either ensure that the number of voltage specifications is equal to the "
267 "number of modes or leave this field as 'None' in the 'MicrowaveModeSpec'."
268 ) Lines 266-274 266 "Please either ensure that the number of voltage specifications is equal to the "
267 "number of modes or leave this field as 'None' in the 'MicrowaveModeSpec'."
268 )
269 if not valid_number_current_specs:
! 270 raise SetupError(
271 f"Given {val.num_current_specs} current specifications, but the number of modes requested is {num_modes}. "
272 "Please either ensure that the number of voltage specifications is equal to the "
273 "number of modes or leave this field as 'None' in the 'MicrowaveModeSpec'."
274 ) tidy3d/plugins/microwave/custom_path_integrals.pyLines 230-295 230 """Current integral comprising one or more disjoint paths"""
231
232 def compute_current(self, em_field: MonitorDataTypes) -> IntegralResultTypes:
233 """Compute current flowing in loop defined by the outer edge of a rectangle."""
! 234 if isinstance(em_field, FieldTimeData) and self.sum_spec == "split":
! 235 raise DataError(
236 "Only frequency domain field data is supported when using the 'split' sum_spec. "
237 "Either switch the sum_spec to 'sum' or supply frequency domain data."
238 )
239
! 240 from tidy3d.components.microwave.path_integral_factory import make_current_integral
241
! 242 current_integrals = [make_current_integral(path_spec) for path_spec in self.path_specs]
243
244 # Initialize arrays with first current term
! 245 first_term = current_integrals[0].compute_current(em_field)
! 246 current_in_phase = xr.zeros_like(first_term)
! 247 current_out_phase = xr.zeros_like(first_term)
248
249 # Get reference phase from first non-zero current
! 250 phase_reference = None
! 251 for path in current_integrals:
! 252 term = path.compute_current(em_field)
! 253 if np.any(term.abs > 0):
! 254 phase_reference = term.angle
! 255 break
! 256 if phase_reference is None:
! 257 raise DataError("Cannot complete calculation of current. No non-zero current found.")
258
259 # Accumulate currents based on phase comparison
! 260 for path in current_integrals:
! 261 term = path.compute_current(em_field)
! 262 if np.all(term.abs == 0):
! 263 continue
264
265 # Compare phase to reference
! 266 phase_diff = term.angle - phase_reference
267 # Wrap phase difference to [-pi, pi]
! 268 phase_diff.values = np.mod(phase_diff.values + np.pi, 2 * np.pi) - np.pi
269
270 # Check phase consistency across frequencies
! 271 self._check_phase_sign_consistency(phase_diff)
272
273 # Add to in-phase or out-of-phase current
! 274 is_in_phase = phase_diff <= np.pi / 2
! 275 current_in_phase += xr.where(is_in_phase, term, 0)
! 276 current_out_phase += xr.where(~is_in_phase, term, 0)
277
! 278 current_in_phase = CurrentIntegralAxisAligned._set_data_array_attributes(current_in_phase)
! 279 current_out_phase = CurrentIntegralAxisAligned._set_data_array_attributes(current_out_phase)
280
! 281 if self.sum_spec == "sum":
! 282 return current_in_phase + current_out_phase
283
284 # Check amplitude consistency across frequencies
! 285 self._check_phase_amplitude_consistency(current_in_phase, current_out_phase)
286
287 # For split mode, return the larger magnitude current
! 288 current = xr.where(
289 abs(current_in_phase) >= abs(current_out_phase), current_in_phase, current_out_phase
290 )
! 291 return CurrentIntegralAxisAligned._set_data_array_attributes(current)
292
293 def _check_phase_sign_consistency(
294 self,
295 phase_difference: Union[FreqDataArray, FreqModeDataArray], |
39318d6
to
469d8ae
Compare
Planning on changing the name to |
That does make more sense. Are there any other impedance specs that might be introduced in future? Just wondering if it would also make sense to expand it to |
Yea I think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20 files reviewed, 8 comments
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile
48c29ea
to
55f9529
Compare
adding support for colocated fields fixing bug for more complicated shapes adding tests, fixing symmetric conditions, fixing for 2D structures fix simulation validator for terminal spec refactor by splitting the path integral specification away from the integral computation, now the path specification handles pretty much everything, except for the final integral computation and results preparation rename auto patch spec to generator type name, fixed tests add test for sim validation and improved doc strings fix regression fix python 3.9 tests reorg of ModeData, the impedance is now calculated by the ModeSolver class avoiding the need for a duplicate terminal spec add more descriptive warnings to composite current integral renamed to MicrowaveModeSpec and allow for supplying manual path specifications for each mode
55f9529
to
8500a6a
Compare
@dbochkov-flexcompute I finally settled on Also in the future, I can see how it might provide additional features/specification fields, like making linear combinations of modes to excite a specified conductor, instead of relying on the set of modes returned by the mode solver. Also combining modes to give a desired polarization direction that is based on the voltage/current paths. |
Calculating impedance automatically:
CompositeCurrentIntegral
) made up of multiple current integrals. The total current flowing in a transmission line is net 0 because of current flowing in and out . To isolate these two terms, we split the currents based on their phase (using an initial reference phase). Then we choose the maximum of the two at the end. We choose the max, because it is possible that some current is flowing back in the PEC boundary or ground plane, which are not totally enclosed by the modal plane and their contribution will be missing.Need at least one small backend change
Todo on:
Greptile Summary
Implements automated impedance calculation for RF transmission lines by introducing a new TerminalSpec system that detects conductors and computes characteristic impedance using voltage/current path integrals.
tidy3d.components.microwave.terminal_spec.TerminalSpec
class for automated transmission line impedance calculation using conductor detectionCompositeCurrentIntegral
to handle complex current flows with phase-based current isolation for accurate impedance measurementsPathSpecGenerator
incomponents/microwave/path_spec_generator.py
to automatically detect conductors and generate appropriate current pathsModeSolver
with characteristic impedance (Z0) calculations integrated with the new terminal systemcomponents/microwave/viz.py
module