Repository navigation
Add ansys plugin: HFSS, Q3D and Q2D through PyAEDT - #787
Conversation
Generic layout import into Ansys Electronics Desktop, driven by the active PDK's LayerStack and a configurable material table: HFSS (lumped CPW ports, air region, eigenmode and S-parameter results), Q3D capacitance matrix and Q2D CPW cross-sections. Includes mocked tests, a docs notebook, CI matrix entry and install notes. Refs gdsfactory#785 Co-authored-by: Niko Savola <niko@gdsfactory.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Niko Savola <niko@gdsfactory.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideAdds a generic gplugins.ansys integration around PyAEDT, with shared layer/material translation, HFSS, Q3D, and Q2D wrappers, mocked coverage, documentation, packaging, and CI integration. The implementation deliberately leaves PDK-specific session and geometry policies to callers while defaulting layer-stack resolution to the active PDK; live AEDT behavior remains unverified. Sequence diagram for the HFSS component simulation workflowsequenceDiagram
participant Caller
participant HFSS
participant LayerStack
participant AEDT
participant Results
Caller->>HFSS: import_component(component)
HFSS->>LayerStack: resolve_layer_stack()
HFSS->>HFSS: layer_stack_to_gds_mapping()
HFSS->>AEDT: import_gds_3d()
HFSS->>HFSS: object_names_to_materials()
HFSS->>AEDT: assign_perfect_e()
Caller->>HFSS: add_lumped_ports(ports, cpw_gap, cpw_width)
HFSS->>AEDT: lumped_port()
Caller->>HFSS: add_air_region(component)
HFSS->>AEDT: create_box()
Caller->>AEDT: analyze_setup()
Caller->>HFSS: get_sparameter_results()
HFSS->>AEDT: get_solution_data()
HFSS-->>Results: Polars DataFrame
Sequence diagram for Q3D capacitance extractionsequenceDiagram
participant Caller
participant Q3D
participant AEDT
participant Results
Caller->>Q3D: import_component(component)
Q3D->>Q3D: layer_stack_to_gds_mapping()
Q3D->>AEDT: import_gds_3d()
Q3D->>Q3D: object_names_to_materials()
Q3D->>AEDT: assign_material()
Caller->>Q3D: assign_nets_from_ports(ports, conductors)
Q3D->>AEDT: auto_identify_nets()
Q3D->>AEDT: assign_net()
Caller->>AEDT: analyze_setup()
Caller->>Q3D: get_capacitance_matrix()
Q3D->>AEDT: get_solution_data()
Q3D-->>Results: Polars DataFrame
Flow diagram for Q2D CPW constructionflowchart TD
Start["create_2d_cpw(width, gap)"] --> Resolve["resolve_layer_stack()"]
Resolve --> Levels["_get_level(substrate_level, conductor_level)"]
Levels --> Materials["add_materials()"]
Materials --> Geometry["create_rectangle() for signal, grounds, substrate"]
Geometry --> Conductors["assign_single_conductor()"]
Conductors --> Mesh["assign_length_mesh()"]
Mesh --> Return["Return created object names"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 15 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="gplugins/ansys/hfss.py" line_range="158-162" />
<code_context>
+ # levels, not to dielectric objects such as the substrate.
+ conductor_objects = [
+ obj_name
+ for obj_name, material in object_names_to_materials(
+ renamed_objects, stack, self.material_properties
+ ).items()
+ if material == "pec"
+ ]
+
+ if conductor_objects:
</code_context>
<issue_to_address>
**Dielectrics keep default material**
When an imported HFSS component contains dielectric geometry, `HFSS.import_component` uses `object_names_to_materials` only to select PEC objects and never registers or assigns the mapped dielectric materials. The imported geometry keeps AEDT’s default material, producing incorrect simulation results.
Register the configured materials and assign each mapped material to its imported object, in addition to assigning PEC boundaries.
</issue_to_address>
### Comment 2
<location path="gplugins/ansys/hfss.py" line_range="131-134" />
<code_context>
+ True if import was successful, False otherwise.
+ """
+ stack = self.resolve_layer_stack(layer_stack)
+ thickness_override = 0.0 if import_as_sheets else None
+ mapping_layers = layer_stack_to_gds_mapping(
+ stack, thickness_override=thickness_override
+ )
+
+ with export_component_to_gds_temp(
</code_context>
<issue_to_address>
**Dielectric layers become sheets**
When `import_as_sheets=True` is used with dielectric layers, `layer_stack_to_gds_mapping` applies the zero-thickness override to every mapped layer, so dielectric and substrate geometry loses its volume in HFSS.
Apply the zero-thickness override only to conductor layers.
</issue_to_address>
### Comment 3
<location path="gplugins/ansys/hfss.py" line_range="220-222" />
<code_context>
+ x_min, y_min = bounds.p1.x, bounds.p1.y
+ dx, dy = bounds.p2.x - x_min, bounds.p2.y - y_min
+
+ region = self.modeler.create_box(
+ origin=[x_min, y_min, -substrate_thickness],
+ sizes=[dx, dy, height + substrate_thickness],
+ name=name,
+ material="vacuum",
</code_context>
<issue_to_address>
**Radiation boundary lacks clearance**
When the returned region is used as a radiation-boundary enclosure, `add_air_region` gives the box the component’s exact x/y bounds, leaving no lateral air clearance. The radiation boundary sits at the component edges, producing an invalid or inaccurate open-region model.
Expand the region’s x/y bounds to provide lateral clearance around the component.
</issue_to_address>
### Comment 4
<location path="gplugins/ansys/hfss.py" line_range="132-145" />
<code_context>
+ """
+ stack = self.resolve_layer_stack(layer_stack)
+ thickness_override = 0.0 if import_as_sheets else None
+ mapping_layers = layer_stack_to_gds_mapping(
+ stack, thickness_override=thickness_override
+ )
+
+ with export_component_to_gds_temp(
+ component, gds_path, prefix="gplugins_hfss_"
+ ) as path:
+ self.modeler.model_units = units
+ existing_objects = set(self.modeler.object_names)
+
+ result = self.hfss.import_gds_3d(
+ input_file=str(path),
+ mapping_layers=mapping_layers,
+ units=units,
+ import_method=0,
+ )
</code_context>
<issue_to_address>
**Imported layers use wrong scale**
When HFSS or Q3D import is called with `units` other than `"um"`, `layer_stack_to_gds_mapping` supplies elevations and thicknesses in micrometers, but the import methods pass them unchanged while forwarding the caller’s units. AEDT interprets the layer heights at the wrong scale, misplacing or resizing the imported geometry.
Convert the layer mapping’s elevations and thicknesses to the requested units before importing.
Also at `gplugins/ansys/q3d.py:87-98`.
</issue_to_address>
### Comment 5
<location path="gplugins/ansys/hfss.py" line_range="258-259" />
<code_context>
+
+ for f_name in freq_names:
+ solution = self.hfss.post.get_solution_data(
+ expressions=f_name, report_category="Eigenmode"
+ )
+ if solution and (freq_hz := _first_real_value(solution)) is not None:
+ results["frequencies"].append(freq_hz / 1e9)
</code_context>
<issue_to_address>
**Eigenmode results use wrong setup**
When the caller requests a setup other than PyAEDT’s default or active setup, `get_eigenmode_results` omits `setup_name` from its `get_solution_data` calls, so AEDT returns data from its selected setup while the result is labeled with the requested setup name.
Pass the requested `setup_name` to each eigenmode `get_solution_data` call.
Also at `gplugins/ansys/hfss.py:264-266`.
</issue_to_address>
### Comment 6
<location path="gplugins/ansys/hfss.py" line_range="71-72" />
<code_context>
+ rect_cx = cx + (cpw_gap / 2) * c
+ rect_cy = cy + (cpw_gap / 2) * s
+
+ origin = [rect_cx - size_x / 2, rect_cy - size_y / 2, 0]
+ int_line = [[cx + cpw_gap * c, cy + cpw_gap * s, 0], [cx, cy, 0]]
+
+ return {"origin": origin, "sizes": [size_x, size_y], "integration_line": int_line}
</code_context>
<issue_to_address>
**Ports sit at wrong elevation**
When the supplied port center has a nonzero z coordinate, `lumped_port_rectangle_from_cpw` ignores `center[2]` and sets the rectangle and integration-line z coordinates to zero. HFSS places the port on the wrong plane, disconnected from its conductor.
Use the supplied center’s z coordinate for the rectangle and both integration-line points.
</issue_to_address>
### Comment 7
<location path="gplugins/ansys/hfss.py" line_range="300" />
<code_context>
+ # Use get_expression_data to get real and imaginary parts
+ _, real_data = solution.get_expression_data(formula="real")
+ _, imag_data = solution.get_expression_data(formula="imag")
+ data[trace] = real_data + 1j * imag_data
+
+ return pl.DataFrame(data)
</code_context>
<issue_to_address>
**S-parameter results cannot form frame**
When a driven solution returns at least one S-parameter trace, `get_sparameter_results` multiplies raw imaginary samples by `1j`, which raises when PyAEDT returns lists; even when the arithmetic succeeds, the resulting complex values cannot be stored in Polars columns. Callers cannot retrieve the results frame.
Convert samples to a supported numeric representation and return the real and imaginary data in Polars-compatible columns.
Also at `gplugins/ansys/hfss.py:302`.
</issue_to_address>
### Comment 8
<location path="gplugins/ansys/hfss.py" line_range="298-301" />
<code_context>
+ data["frequency_ghz"] = np.array(solution.primary_sweep_values)
+
+ # Use get_expression_data to get real and imaginary parts
+ _, real_data = solution.get_expression_data(formula="real")
+ _, imag_data = solution.get_expression_data(formula="imag")
+ data[trace] = real_data + 1j * imag_data
+
+ return pl.DataFrame(data)
</code_context>
<issue_to_address>
**Empty traces break result extraction**
When a trace exists but has no samples for the requested setup and sweep, `get_sparameter_results` proceeds from a truthy solution to `get_expression_data`; missing or empty values then break the real/imaginary combination or produce inconsistent column lengths. Callers cannot retrieve a valid result DataFrame.
Detect missing or empty expression samples and handle them before combining data or building the DataFrame.
</issue_to_address>
### Comment 9
<location path="gplugins/ansys/hfss.py" line_range="287" />
<code_context>
- id: ruff-format
- files: ^gplugins/(emode|femwell|gmsh|meow|sax|tidy3d|klayout|vlsir)/
+ files: ^gplugins/(ansys|emode|femwell|gmsh|meow|sax|tidy3d|klayout|vlsir)/
# - repo: https://github.com/pre-commit/mirrors-mypy
# rev: "v1.16.0"
</code_context>
<issue_to_address>
**Sweep frequencies have wrong units**
When the selected sweep reports primary values in units other than GHz, `get_sparameter_results` copies `primary_sweep_values` unchanged into `frequency_ghz`, so callers interpret values in the sweep’s units as GHz and receive incorrectly scaled frequencies.
Convert the primary sweep values to GHz before storing them in `frequency_ghz`.
Also at `gplugins/ansys/hfss.py:296`.
</issue_to_address>
### Comment 10
<location path="gplugins/ansys/q3d.py" line_range="355-357" />
<code_context>
+ conductor_thickness = 2.0
+ conductor_material = cast(str, conductor.material)
+
+ if ground_width is None:
+ ground_width = 10.0 * cpw_gap
+
+ self.add_materials()
+ self.modeler.model_units = units
</code_context>
<issue_to_address>
**Invalid ground width is accepted**
When a caller supplies `ground_width <= 0`, `create_2d_cpw` uses the invalid width to create ground rectangles, producing degenerate or invalid geometry instead of a clear input error.
Reject nonpositive `ground_width` values before creating geometry.
</issue_to_address>
### Comment 11
<location path="gplugins/ansys/base.py" line_range="316-319" />
<code_context>
+ List of renamed object names.
+ """
+ num_to_name = {}
+ for name, level in layer_stack.layers.items():
+ layer_num = _get_layer_number_from_level(level)
+ if layer_num is not None and layer_num not in num_to_name:
+ num_to_name[layer_num] = name
+
+ renamed_objects = []
</code_context>
<issue_to_address>
**Substrate resolves to vacuum**
When a vacuum level precedes a non-vacuum level that shares its GDS layer number, `rename_imported_objects` and `object_names_to_materials` keep the first stack level for each GDS layer number, including vacuum levels skipped by the import mapping. Q3D therefore assigns the imported substrate the vacuum material and produces incorrect extraction results.
Resolve shared GDS layer numbers to the imported non-vacuum level rather than the first stack level.
Also at `gplugins/ansys/base.py:372-375`.
</issue_to_address>
### Comment 12
<location path="gplugins/ansys/hfss.py" line_range="158-162" />
<code_context>
+ # levels, not to dielectric objects such as the substrate.
+ conductor_objects = [
+ obj_name
+ for obj_name, material in object_names_to_materials(
+ renamed_objects, stack, self.material_properties
+ ).items()
+ if material == "pec"
+ ]
+
+ if conductor_objects:
</code_context>
<issue_to_address>
**Failed imports leave partial geometry**
When a post-import material lookup or AEDT assignment fails, `HFSS.import_component` has already added geometry when material lookup or a later assignment raises, leaving objects with default or partial assignments in the design. Retrying imports the component again instead of repairing those objects.
Clean up the newly imported objects when post-import lookup or assignment fails.
Also at `gplugins/ansys/q3d.py:111-119`.
</issue_to_address>
### Comment 13
<location path="gplugins/ansys/_mocks.py" line_range="170-173" />
<code_context>
+ self.requests.append((expressions, kwargs))
+ if not self.solved or expressions is None:
+ return False
+ for category in self.quantities.values():
+ if expressions in category:
+ value = category[expressions]
+ values = [] if value is None else [value]
+ solution = MockSolutionData(expressions, values, self.unit)
+ self.solutions.append(solution)
</code_context>
<issue_to_address>
**Wrong AEDT queries pass mocked tests**
When the result method supplies an incorrect `report_category` or `setup_sweep_name`, `MockPost.get_solution_data` selects canned data by expression alone and ignores its recorded keyword arguments, so `test_get_eigenmode_results_reads_real_values` and `test_get_capacitance_matrix_converts_the_reported_unit` pass even when the code requests the wrong report category or setup.
Assert the recorded query arguments in these tests, or make `MockPost` return canned data only for the expected arguments.
Also at `gplugins/ansys/tests/test_ansys.py:331`, `gplugins/ansys/tests/test_ansys.py:395`.
</issue_to_address>
### Comment 14
<location path="gplugins/ansys/hfss.py" line_range="261" />
<code_context>
+ expressions=f_name, report_category="Eigenmode"
+ )
+ if solution and (freq_hz := _first_real_value(solution)) is not None:
+ results["frequencies"].append(freq_hz / 1e9)
+
+ for q_name in q_names:
</code_context>
<issue_to_address>
**Eigenfrequencies assume hertz**
When pyAEDT returns an eigenmode frequency in units other than Hz, `get_eigenmode_results` divides the reported value by 1e9 without checking its unit. When AEDT returns the frequency in GHz, the method reports a value one billion times too small and callers see incorrect eigenfrequencies.
Read the expression's reported unit and convert the frequency to GHz based on that unit.
</issue_to_address>
### Comment 15
<location path="gplugins/ansys/hfss.py" line_range="254" />
<code_context>
+ quantities_category="Eigen Q"
+ )
+
+ results = {"frequencies": [], "q_factors": [], "setup": setup_name}
+
+ for f_name in freq_names:
</code_context>
<issue_to_address>
**Eigenmode results are not a DataFrame**
`get_eigenmode_results` returns a dictionary of lists, so callers following the advertised polars-DataFrame contract cannot use DataFrame operations on its result and receive a different API from the S-parameter method.
Return the eigenmode frequencies and Q factors in the advertised polars DataFrame format, or correct the documented contract if a dictionary is intended.
</issue_to_address>Sourcery assessment
Approval pending. 15 findings to address first.
Blocking findings: gplugins/ansys/hfss.py:162, gplugins/ansys/hfss.py:134, gplugins/ansys/hfss.py:222, gplugins/ansys/hfss.py:145, gplugins/ansys/hfss.py:259, and 10 more
| region = self.modeler.create_box( | ||
| origin=[x_min, y_min, -substrate_thickness], | ||
| sizes=[dx, dy, height + substrate_thickness], |
There was a problem hiding this comment.
🟠 High · Radiation boundary lacks clearance
When the returned region is used as a radiation-boundary enclosure, add_air_region gives the box the component’s exact x/y bounds, leaving no lateral air clearance. The radiation boundary sits at the component edges, producing an invalid or inaccurate open-region model.
Expand the region’s x/y bounds to provide lateral clearance around the component.
Prompt for AI agents
In `gplugins/ansys/hfss.py` at lines 220-222:
**Radiation boundary lacks clearance**
When the returned region is used as a radiation-boundary enclosure, `add_air_region` gives the box the component’s exact x/y bounds, leaving no lateral air clearance. The radiation boundary sits at the component edges, producing an invalid or inaccurate open-region model.
Expand the region’s x/y bounds to provide lateral clearance around the component.| expressions=f_name, report_category="Eigenmode" | ||
| ) |
There was a problem hiding this comment.
🟠 High · Eigenmode results use wrong setup
When the caller requests a setup other than PyAEDT’s default or active setup, get_eigenmode_results omits setup_name from its get_solution_data calls, so AEDT returns data from its selected setup while the result is labeled with the requested setup name.
Pass the requested setup_name to each eigenmode get_solution_data call.
Also at gplugins/ansys/hfss.py:264-266.
Prompt for AI agents
In `gplugins/ansys/hfss.py` at lines 258-259:
**Eigenmode results use wrong setup**
When the caller requests a setup other than PyAEDT’s default or active setup, `get_eigenmode_results` omits `setup_name` from its `get_solution_data` calls, so AEDT returns data from its selected setup while the result is labeled with the requested setup name.
Pass the requested `setup_name` to each eigenmode `get_solution_data` call.
Also at `gplugins/ansys/hfss.py:264-266`.| for obj_name, material in object_names_to_materials( | ||
| renamed_objects, stack, self.material_properties | ||
| ).items() | ||
| if material == "pec" | ||
| ] |
There was a problem hiding this comment.
🟡 Medium · Failed imports leave partial geometry
When a post-import material lookup or AEDT assignment fails, HFSS.import_component has already added geometry when material lookup or a later assignment raises, leaving objects with default or partial assignments in the design. Retrying imports the component again instead of repairing those objects.
Clean up the newly imported objects when post-import lookup or assignment fails.
Also at gplugins/ansys/q3d.py:111-119.
Prompt for AI agents
In `gplugins/ansys/hfss.py` at lines 158-162:
**Failed imports leave partial geometry**
When a post-import material lookup or AEDT assignment fails, `HFSS.import_component` has already added geometry when material lookup or a later assignment raises, leaving objects with default or partial assignments in the design. Retrying imports the component again instead of repairing those objects.
Clean up the newly imported objects when post-import lookup or assignment fails.
Also at `gplugins/ansys/q3d.py:111-119`.| expressions=f_name, report_category="Eigenmode" | ||
| ) | ||
| if solution and (freq_hz := _first_real_value(solution)) is not None: | ||
| results["frequencies"].append(freq_hz / 1e9) |
There was a problem hiding this comment.
🟠 High · Eigenfrequencies assume hertz
When pyAEDT returns an eigenmode frequency in units other than Hz, get_eigenmode_results divides the reported value by 1e9 without checking its unit. When AEDT returns the frequency in GHz, the method reports a value one billion times too small and callers see incorrect eigenfrequencies.
Read the expression's reported unit and convert the frequency to GHz based on that unit.
Prompt for AI agents
In `gplugins/ansys/hfss.py` at line 261:
**Eigenfrequencies assume hertz**
When pyAEDT returns an eigenmode frequency in units other than Hz, `get_eigenmode_results` divides the reported value by 1e9 without checking its unit. When AEDT returns the frequency in GHz, the method reports a value one billion times too small and callers see incorrect eigenfrequencies.
Read the expression's reported unit and convert the frequency to GHz based on that unit.There was a problem hiding this comment.
🔵 Needs a closer look
Solver integration remains unverified on live AEDT and several geometry, unit, material, and result-extraction correctness issues remain.
10 open findings
Port Z coordinate is ignored · New Layer stack units are not converted or validated · New Requested setup name is ignored in result queries · New Layer mapping units are not converted or validated · New Unit inference is ambiguous and scales ports incorrectly · New Repeated ports on one conductor are assigned incorrectly · New Zero thickness override collapses dielectric layers · New Air box lacks lateral padding · New Sweep frequencies are mislabeled as GHz · New Nonpositive ground width bypasses validation · New
What changed in this PR
Adds a reusable PyAEDT-based Ansys plugin supporting HFSS, Q3D, and Q2D workflows across PDKs.
Changes:
- Adds shared AEDT utilities and solver wrappers.
- Adds mocked tests and an example notebook.
- Integrates the plugin into packaging, CI, documentation, and release notes.
| File | Description |
|---|---|
README.md |
Documents Ansys capabilities and installation. |
pyproject.toml |
Adds the ansys dependency extra. |
notebooks/ansys_01_aedt.ipynb |
Demonstrates HFSS, Q3D, and Q2D workflows. |
gplugins/ansys/base.py |
Implements shared layer, material, export, and session utilities. |
gplugins/ansys/hfss.py |
Implements HFSS import, ports, regions, and result extraction. |
gplugins/ansys/q3d.py |
Implements Q3D and Q2D workflows. |
gplugins/ansys/__init__.py |
Exposes the public plugin API. |
gplugins/ansys/_mocks.py |
Provides PyAEDT test doubles. |
gplugins/ansys/tests/test_ansys.py |
Adds mocked solver-wrapper tests. |
gplugins/ansys/tests/__init__.py |
Initializes the test package. |
docs/zensical.yml |
Adds the notebook to documentation navigation. |
.pre-commit-config.yaml |
Includes Ansys code in Ruff hooks. |
.github/workflows/test_code.yml |
Adds Ansys to the CI matrix. |
.changelog.d/787.added |
Records the new plugin. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| origin = [rect_cx - size_x / 2, rect_cy - size_y / 2, 0] | ||
| int_line = [[cx + cpw_gap * c, cy + cpw_gap * s, 0], [cx, cy, 0]] |
| Returns: | ||
| True if import was successful, False otherwise. | ||
| """ | ||
| stack = self.resolve_layer_stack(layer_stack) |
| for f_name in freq_names: | ||
| solution = self.hfss.post.get_solution_data( | ||
| expressions=f_name, report_category="Eigenmode" | ||
| ) | ||
| if solution and (freq_hz := _first_real_value(solution)) is not None: | ||
| results["frequencies"].append(freq_hz / 1e9) | ||
|
|
||
| for q_name in q_names: | ||
| solution = self.hfss.post.get_solution_data( | ||
| expressions=q_name, report_category="Eigenmode" | ||
| ) |
| Raises: | ||
| RuntimeError: If GDS import fails. | ||
| """ | ||
| stack = self.resolve_layer_stack(layer_stack) |
| scale_factor = min( | ||
| (10**p for p in range(-3, 5)), | ||
| key=lambda s: min(dist_to_bbox(px0, py0, b, s) for b in bboxes.values()), | ||
| ) |
| for port in ports: | ||
| px, py = float(port.center[0]), float(port.center[1]) | ||
| available_objs = [obj for obj in bboxes if obj not in used_objects] | ||
| if not available_objs: | ||
| break |
| thickness_override = 0.0 if import_as_sheets else None | ||
| mapping_layers = layer_stack_to_gds_mapping( | ||
| stack, thickness_override=thickness_override | ||
| ) |
| region = self.modeler.create_box( | ||
| origin=[x_min, y_min, -substrate_thickness], | ||
| sizes=[dx, dy, height + substrate_thickness], | ||
| name=name, | ||
| material="vacuum", | ||
| ) |
| if "frequency_ghz" not in data: | ||
| data["frequency_ghz"] = np.array(solution.primary_sweep_values) |
| if ground_width is None: | ||
| ground_width = 10.0 * cpw_gap |
|
Critical review (automated, Fable model) Scope: full diff at HighH1. H2. H3. MediumM1. Docs claim is false: the notebook does not run in docs CI, it errors silently — M2. M3. Capacitance unit fallback silently mis-scales — M4. M5. The untested methods are exactly the broken ones Low
Claims in the description
Verdict: NOT MERGEABLE as is. H1-H3 produce silently wrong simulation inputs/outputs; M1 means the only CI-visible evidence for the notebook is a traceback page. Fix H1-H3 and M1 with tests, then re-review. |
- Q3D.assign_nets_from_ports: drop the scale-factor search that misassigned nets with a port at the origin; convert bounding boxes from the modeler's length unit to um, raise on unknown units or ports outside the conductors, and skip (with a warning) a port whose conductor already carries a net. - HFSS.get_sparameter_results: read S(i,j) traces, not dB(S(i,j)), and return float re()/im() columns with the sweep converted to GHz. - HFSS.import_component: add the material table and assign dielectric materials; import_as_sheets flattens conductor levels only; raise on import failure as Q3D does. Both importers now require units="um". - Remove the no-op mesh_order assignments; add_substrate adds its material. - get_capacitance_matrix raises on an unknown unit and accepts aF. - rename_imported_objects no longer swallows AEDT errors. - lumped ports reject orientation=None with the port's name; add_air_region takes a lateral_margin; unresolvable layer levels are logged. - Move _mocks.py under tests/; pyaedt>=1.8.0; lock the ansys extra. - Docs CI installs the ansys extra and requires the notebook to execute. - Tests for net assignment, S-parameters, HFSS import, ports, air region, substrate, capacitance units and renaming. Co-authored-by: Niko Savola <niko@gdsfactory.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Replies to the critical review, as of c4bc6e9. Live AEDT behaviour is still UNVERIFIED for everything below; checks are against pyaedt 1.8.0 sources and mocked tests (58 pass, 23 new). Each new test was confirmed to fail against dcfff19. High
Medium
Low
Description claims: "eigenmode returns a dict" is unchanged (it does return a dict). "Ported from working PDK code" is still UNVERIFIED end to end. |
Sourcery withdrew this approval because the latest commits introduced blocking findings.
Resolve conflicts with the comsol plugin by keeping both plugins in the CI matrix, pre-commit ruff scope, README, docs nav and extras, then regenerate uv.lock. Co-authored-by: Joaquin Matres <4514346+joamatab@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>


Part 2 of #785: an
ansysplugin that generalizes the HFSS/Q3D/Q2D helpers from the Quantum RF PDK so any PDK can use them through PyAEDT.What's in it
gplugins.ansys.baselayer_stack_to_gds_mapping, with the active PDK'sLayerStackas the default.object_names_to_materialsandadd_materials_to_aedt, which take a configurablematerial_propertiestable. Infinite permittivity maps topec.AEDTBasewrapper.gplugins.ansys.hfss.HFSSgplugins.ansys.q3dQ3D: import plus the capacitance matrix.Q2D.create_2d_cpw(width, gap, substrate_level=..., conductor_level=...).ansys_01_aedt: the geometry and mapping steps run in CI. The licensed AEDT cells sit behindRUN_AEDT = False.ansysextra (pyaedt,polars), a CI matrix entry, the ruff pre-commit scope and README install notes.PDK-specific behaviour stays in the PDK: mask inversion, singleton sessions and default stacks. The PDK wraps these classes.
Caveats
simulations.Requested by @nikosavola
🤖 Generated with Claude Code
Summary by Sourcery
Add a reusable Ansys AEDT plugin for HFSS, Q3D, and Q2D simulations from gdsfactory components.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests: