Skip to content

Add ansys plugin: HFSS, Q3D and Q2D through PyAEDT - #787

Merged
joamatab merged 4 commits into
gdsfactory:mainfrom
jackgdsf:ansys-plugin
Oct 12, 2026
Merged

joamatab merged 4 commits into
gdsfactory:mainfrom
jackgdsf:ansys-plugin

Conversation

@jackgdsf

@jackgdsf jackgdsf commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Part 2 of #785: an ansys plugin 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.base
    • layer_stack_to_gds_mapping, with the active PDK's LayerStack as the default.
    • object_names_to_materials and add_materials_to_aedt, which take a configurable material_properties table. Infinite permittivity maps to pec.
    • The AEDTBase wrapper.
  • gplugins.ansys.hfss.HFSS
    • GDS import with PerfectE assignment, lumped CPW ports and the air region.
    • Eigenmode and S-parameter results returned as polars DataFrames.
  • gplugins.ansys.q3d
    • Q3D: import plus the capacitance matrix.
    • Q2D.create_2d_cpw(width, gap, substrate_level=..., conductor_level=...).
  • Tests: 35 tests against mocked PyAEDT objects, so no Ansys install is needed.
  • Docs notebook ansys_01_aedt: the geometry and mapping steps run in CI. The licensed AEDT cells sit behind RUN_AEDT = False.
  • Packaging: the ansys extra (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

  • Unverified on a live AEDT install. The PyAEDT calls are ported from working PDK code, but @nikosavola will verify them on real software.
  • A human must review this code before merge.
  • I don't have permission to set labels here. Please add 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:

  • Add PyAEDT-backed HFSS, Q3D Extractor, and 2D Extractor wrappers for importing gdsfactory components and running common RF and parasitic extraction workflows.
  • Provide layer-stack/material mapping, CPW lumped-port geometry, air and substrate setup, net assignment, and structured simulation result extraction.
  • Add an Ansys documentation notebook covering geometry preparation and optional licensed AEDT workflows.

Enhancements:

  • Support configurable material properties, active-PDK layer-stack defaults, conductor handling, unit conversion, and robust headless AEDT session helpers.
  • Return HFSS and Q3D results in convenient Polars-compatible tabular formats.

Build:

  • Add the ansys optional dependency extra with PyAEDT and Polars and update the lockfile.

CI:

  • Run Ansys plugin tests and documentation builds in CI using mocked PyAEDT objects without requiring an Ansys installation.
  • Extend formatting and linting coverage to the Ansys plugin.

Documentation:

  • Document Ansys plugin capabilities and installation requirements in the README and generated documentation navigation.

Tests:

  • Add comprehensive mocked-PyAEDT tests for layer mapping, material assignment, imports, ports, nets, result extraction, and Q2D geometry.

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>
@sourcery-ai

sourcery-ai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds 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 workflow

sequenceDiagram
    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
Loading

Sequence diagram for Q3D capacitance extraction

sequenceDiagram
    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
Loading

Flow diagram for Q2D CPW construction

flowchart 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"]
Loading

File-Level Changes

Change Details Files
Introduces shared PyAEDT integration utilities for translating gdsfactory layer stacks and materials into AEDT-compatible geometry and assignments.
  • Map GDS layer numbers to elevations and thicknesses, including vacuum filtering and collision validation/merging.
  • Resolve layer-stack objects to dielectric materials or PEC, with configurable material properties and AEDT material creation.
  • Provide component-to-temporary-GDS export, imported-object renaming, substrate creation, view/session safeguards, and a reusable AEDT wrapper base.
gplugins/ansys/__init__.py
gplugins/ansys/base.py
Adds an HFSS wrapper for importing components, configuring common CPW simulation geometry, and reading solver results.
  • Import 3D GDS geometry or sheets using layer-stack mappings and assign PerfectE only to conductor objects.
  • Generate orientation-aware lumped CPW port rectangles and add air regions/substrates.
  • Return eigenmode data and driven S-parameters, including polars DataFrame output.
gplugins/ansys/hfss.py
Adds Q3D and Q2D extraction workflows based on layer-stack-driven geometry.
  • Import Q3D conductors, assign dielectric/PEC materials, map ports to signal nets, and expose capacitance matrices with unit conversion.
  • Construct a Q2D CPW cross-section from named substrate/conductor levels, assign signal/reference conductors, and configure mesh settings.
gplugins/ansys/q3d.py
Adds comprehensive mocked tests for the new Ansys integration without requiring an AEDT installation.
  • Cover layer mapping edge cases, material handling, imports, port geometry, session helpers, result extraction, unit conversion, net assignment inputs, and Q2D validation.
  • Provide minimal PyAEDT stand-ins for applications, modelers, materials, post-processing, boundaries, and mesh APIs.
gplugins/ansys/_mocks.py
gplugins/ansys/tests/test_ansys.py
Documents and packages the Ansys plugin and integrates it into project automation.
  • Add the ansys optional dependency extra, README installation guidance, documentation navigation, and an AEDT notebook with licensed cells disabled by default.
  • Include Ansys in the plugin CI matrix and Ruff pre-commit file scopes.
pyproject.toml
README.md
docs/zensical.yml
notebooks/ansys_01_aedt.ipynb
.github/workflows/test_code.yml
.pre-commit-config.yaml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

sourcery-ai[bot]
sourcery-ai Bot previously approved these changes Oct 10, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sourcery assessment

Approved.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread gplugins/ansys/hfss.py Outdated
Comment thread gplugins/ansys/hfss.py Outdated
Comment thread gplugins/ansys/hfss.py
Comment on lines +220 to +222
region = self.modeler.create_box(
origin=[x_min, y_min, -substrate_thickness],
sizes=[dx, dy, height + substrate_thickness],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 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.

Comment thread gplugins/ansys/hfss.py
Comment thread gplugins/ansys/hfss.py
Comment on lines +258 to +259
expressions=f_name, report_category="Eigenmode"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 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`.

Comment thread gplugins/ansys/base.py Outdated
Comment thread gplugins/ansys/hfss.py Outdated
Comment on lines +158 to +162
for obj_name, material in object_names_to_materials(
renamed_objects, stack, self.material_properties
).items()
if material == "pec"
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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`.

Comment thread gplugins/ansys/tests/_mocks.py
Comment thread gplugins/ansys/hfss.py
expressions=f_name, report_category="Eigenmode"
)
if solution and (freq_hz := _first_real_value(solution)) is not None:
results["frequencies"].append(freq_hz / 1e9)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 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.

Comment thread gplugins/ansys/hfss.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 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
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.

Comment thread gplugins/ansys/hfss.py Outdated
Comment on lines +71 to +72
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]]
Comment thread gplugins/ansys/hfss.py
Returns:
True if import was successful, False otherwise.
"""
stack = self.resolve_layer_stack(layer_stack)
Comment thread gplugins/ansys/hfss.py
Comment on lines +256 to +266
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"
)
Comment thread gplugins/ansys/q3d.py
Raises:
RuntimeError: If GDS import fails.
"""
stack = self.resolve_layer_stack(layer_stack)
Comment thread gplugins/ansys/q3d.py Outdated
Comment on lines +173 to +176
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()),
)
Comment thread gplugins/ansys/q3d.py Outdated
Comment on lines +178 to +182
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
Comment thread gplugins/ansys/hfss.py Outdated
Comment on lines +131 to +134
thickness_override = 0.0 if import_as_sheets else None
mapping_layers = layer_stack_to_gds_mapping(
stack, thickness_override=thickness_override
)
Comment thread gplugins/ansys/hfss.py
Comment on lines +220 to +225
region = self.modeler.create_box(
origin=[x_min, y_min, -substrate_thickness],
sizes=[dx, dy, height + substrate_thickness],
name=name,
material="vacuum",
)
Comment thread gplugins/ansys/hfss.py Outdated
Comment on lines +294 to +295
if "frequency_ghz" not in data:
data["frequency_ghz"] = np.array(solution.primary_sweep_values)
Comment thread gplugins/ansys/q3d.py
Comment on lines +355 to +356
if ground_width is None:
ground_width = 10.0 * cpw_gap
@jackgdsf

Copy link
Copy Markdown
Contributor Author

Critical review (automated, Fable model)

Scope: full diff at dcfff19, read against pyaedt 1.8.0 and gdsfactory 9.51.0 sources; the 35 mocked tests were run locally (pass). Live AEDT behaviour is UNVERIFIED throughout, as the description itself says. Repros below were run against the PR code with stand-in objects.

High

H1. Q3D.assign_nets_from_ports assigns the wrong conductor when a port sits at the origin — gplugins/ansys/q3d.py:173-192
scale_factor is chosen by min(...) over 10**-3 .. 10**4 using only the first port. With the first port at (0, 0) every scale gives distance 0 (the bbox shrinks towards the origin), so min returns the first candidate, 1e-3. Every other port is then measured against bboxes scaled by 1e-3, which collapses them towards the origin, and the object with the largest coordinates wins, not the nearest.
Repro: ports o1=(0,0), o2=(300,0); conductors at x=[0,10], [310,320], [1000,1010] (um). Result: o2 -> [1000,1010], 700 um away, while the conductor 10 um away is skipped. Moving o1 to (5,0) gives the correct answer. The notebook's own CPW puts o1 at (0,0).
Fix: drop the scale search. import_component already sets modeler.model_units = units, so port coordinates and bounding_box are in the same units; if a unit conversion is needed, apply the explicit factor for units.

H2. HFSS.get_sparameter_results returns dB magnitudes labelled as complex S-parameters — gplugins/ansys/hfss.py:285-300
get_traces_for_plot() is called with its default category="dB(S" (pyaedt analysis.py:721), so the expressions are dB(S(1,1)), dB(S(1,2)), ... Taking formula="real" and formula="imag" of a dB quantity gives the dB value and 0. The DataFrame therefore holds dB + 0j under column names the docstring describes as "complex values". Anything downstream that treats them as S (e.g. abs, phase, conversion to Y/Z) is wrong. No test covers this method.
Fix: get_traces_for_plot(category="S") (expressions S(1,1)), then real/imag; name the column after the real expression. Also note polars stores complex as Object dtype, which blocks most column operations; two float columns (re/im) or magnitude/phase would be more useful.

H3. HFSS.import_component leaves dielectric objects at the project default material and never adds materials — gplugins/ansys/hfss.py:149-170
object_names_to_materials is computed only to filter conductors; dielectric objects (e.g. a substrate level on its own GDS layer) are imported and left with HFSS's default material (vacuum). add_materials() is never called in the HFSS path, so stack materials such as si/Nb do not even exist in the project. The docstring of object_names_to_materials (base.py:363-368) says the point of failing closed is to avoid "silently leaving objects with the project default material"; the HFSS path does exactly that. Repro with the test stack plus a (99,0) substrate polygon: materials added = [], materials assigned = {}, only PEC_3D: ['M1']. Additionally, with import_as_sheets=True the thickness_override=0.0 is applied to every level, so a dielectric substrate becomes a zero-thickness sheet.
Fix: mirror Q3D.import_component (q3d.py:111-119): add_materials(), assign_material per material; apply the sheet override to conductor levels only.

Medium

M1. Docs claim is false: the notebook does not run in docs CI, it errors silently — .github/workflows/pages.yml:89, notebooks/ansys_01_aedt.ipynb cell 1
The PR says "the geometry and mapping steps run in CI". pages.yml installs --extra dev docs elmer emode femwell meow sax tidy3d klayout vlsir, not ansys; make nbdocs runs this notebook in the [exec] branch with allow_errors=True and stderr to /dev/null. The "Build docs" artifact of this very PR renders notebooks/ansys_01_aedt/index.html with ModuleNotFoundError: No module named 'polars' (7 traceback markers in the page). Nothing after cell 1 executed.
Fix: add --extra ansys to pages.yml; add ansys_01_aedt to REQUIRED_PATTERNS in the Makefile so a regression fails the build instead of shipping a traceback page.

M2. mesh_order assignments are no-ops — gplugins/ansys/base.py:503, gplugins/ansys/hfss.py:226
Object3d has no mesh_order property in pyaedt 1.8.0 (hasattr(Object3d, "mesh_order") is False), so substrate.mesh_order = 4 and region.mesh_order = 99 set a Python attribute on the wrapper object and send nothing to AEDT. The code reads as if it controls mesh/overlap priority; it does not. Remove it or implement through a real pyaedt call.

M3. Capacitance unit fallback silently mis-scales — gplugins/ansys/q3d.py:237-246
An unrecognised unit string (e.g. aF, which Q3D does report for small couplings, or a unit with surrounding whitespace) falls back to 1e-12. A value in aF would come back 1e6 too large with no warning. Raise on unknown units or convert via pyaedt's unit_converter.

M4. rename_imported_objects is unnecessary and swallows every error — gplugins/ansys/base.py:305-337
pyaedt's import_gds_3d already accepts {layer: [(elevation, thickness), "layer_name"]} (analysis_3d.py:1518, 1561), so the importer can name objects after the stack levels directly. Instead the plugin imports as signal<N>, then renames with one ChangeProperty round trip per object inside except Exception: pass, which also hides a dead gRPC session, after which object_names_to_materials silently falls back to prefix matching. Build the mapping with names and delete the rename step.

M5. The untested methods are exactly the broken ones
get_sparameter_results, add_lumped_ports, add_air_region, add_substrate and assign_nets_from_ports have no tests at all; H1-H3 and M2 live in them. "35 tests" measures the mapping helpers, not the AEDT workflow. At minimum add unit tests for H1 (the origin case) and H2 (expression names) when fixing them.

Low

  • gplugins/ansys/hfss.py:57: port.orientation % 90 raises TypeError for ports with orientation=None (common on electrical ports). Validate and raise a ValueError naming the port.
  • gplugins/ansys/hfss.py:149,172: HFSS.import_component returns False on import failure and continues; Q3D.import_component raises RuntimeError. Callers of HFSS then run add_lumped_ports against nothing. Pick one behaviour (raise).
  • gplugins/ansys/base.py:172-173: a level whose GDS layer number cannot be resolved is dropped from the mapping with no log line, so its geometry is silently not imported.
  • gplugins/ansys/hfss.py:216-225: add_air_region uses the component bbox with zero lateral margin, so a radiation boundary sits flush on the ground-plane edges. Expose a margin.
  • gplugins/ansys/_mocks.py ships inside the package; it belongs under tests/.
  • pyproject.toml: pyaedt>=0.15.0 is UNVERIFIED. Everything above was checked against 1.8.0; get_expression_data, desktop_class.non_graphical, lumped_port(...) and the import_gds_3d list form were not checked at 0.15.
  • CI: pre-commit is red on this head, but from lychee failing on an unrelated external link (simpetus.com); not caused by this PR.

Claims in the description

  • "35 tests against mocked PyAEDT objects" — verified, pass locally.
  • "Infinite permittivity maps to pec" — verified (base.py:405-406).
  • "the geometry and mapping steps run in CI" — false, see M1.
  • "Eigenmode and S-parameter results returned as polars DataFrames" — eigenmode returns a dict; S-parameters are dB magnitudes, see H2.
  • "PyAEDT calls are ported from working PDK code" — UNVERIFIED; H1/H2/M2 suggest at least some paths were never exercised end to end.

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>
@jackgdsf

Copy link
Copy Markdown
Contributor Author

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

  • H1 Fixed. Scale search removed. Bounding boxes are converted from modeler.model_units to µm by an explicit table; unknown units and ports outside the conductors' extent raise. A port whose nearest conductor already has a net is skipped with a warning instead of being pushed onto another conductor. Your repro is test_assign_nets_from_ports_with_a_port_at_the_origin.
  • H2 Fixed. get_traces_for_plot(category="S"). Each trace is now two Float64 columns, re(S(i,j)) and im(S(i,j)), and the sweep is converted to GHz from units_sweeps.
  • H3 Fixed. HFSS import calls add_materials() and assigns each dielectric its material. Conductors still get PerfectE. import_as_sheets flattens conductor levels only, via a new override_materials argument on layer_stack_to_gds_mapping.

Medium

  • M1 Fixed. pages.yml installs --extra ansys, and ansys_01_aedt is in REQUIRED_PATTERNS. The notebook executes cleanly with allow_errors=False locally. uv.lock now includes the extra (additions only).
  • M2 Fixed. Both mesh_order assignments removed.
  • M3 Fixed. Unknown or missing units raise. aF added.
  • M4 Partly. The except Exception: pass is gone, so AEDT errors propagate; a refused rename keeps its old name and is still resolved. I kept the rename step rather than the [tuple, name] mapping form, because how AEDT names the imported objects from that form is UNVERIFIED without a live session.
  • M5 Fixed for the mockable surface. New tests cover assign_nets_from_ports, get_sparameter_results, HFSS import (dielectrics, sheets, failure), add_lumped_ports, add_air_region, add_substrate, capacitance units and renaming.

Low

  • orientation None: Fixed. Raises ValueError naming the port.
  • HFSS False vs Q3D raise: Fixed. HFSS raises RuntimeError. Both importers also reject units != "um", because the layer stack is in µm.
  • Levels dropped silently: Fixed. Logged as a warning.
  • Zero lateral margin: Fixed. add_air_region(..., lateral_margin=0.0) is keyword-only; the default keeps the old box.
  • _mocks.py in package: Fixed. Moved to gplugins/ansys/tests/_mocks.py, which is excluded from name-tests-test.
  • pyaedt bound: Fixed by raising it to pyaedt>=1.8.0, the version checked against. Older versions were not tested.
  • lychee: Not changed. It is still red only on the unrelated simpetus.com and jpier.org links.

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-ai
sourcery-ai Bot dismissed their stale review October 10, 2026 11:38

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>
@joamatab joamatab added the enhancement New feature or request label Oct 12, 2026
@joamatab
joamatab merged commit 858c5a7 into gdsfactory:main Oct 12, 2026
18 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants