Skip to content

Conversation

dmarek-flex
Copy link
Contributor

@dmarek-flex dmarek-flex commented Oct 9, 2025

Greptile Overview

Updated On: 2025-10-09 15:24:29 UTC

Summary

This PR fixes a bug in the WavePort mode handling within antenna metrics calculations in the Tidy3D S-matrix plugin. The core issue was that WavePort objects have associated mode indices that need to be properly tracked and used when generating task names for simulation data lookup.

The changes span multiple components of the S-matrix terminal component modeling system:

  1. Base Component Modeler: The get_task_name method now automatically uses a WavePort's inherent mode_index attribute when no explicit mode index is provided, ensuring consistent task naming.

  2. Terminal Data Interface: The get_antenna_metrics_data method signature was updated to use NetworkIndex type instead of generic str for port amplitude keys, improving type safety and API clarity.

  3. Antenna Analysis: Enhanced the antenna metrics calculation to properly handle WavePort mode indices in both default port amplitude creation and simulation data retrieval, preventing KeyError exceptions.

  4. Testing Infrastructure: Added parameterized testing to cover both lumped and wave port configurations, and increased simulation domain size for coaxial structures to ensure accurate mode calculations.

These changes align the antenna analysis functionality with the established NetworkIndex pattern used elsewhere in the codebase, where individual excitations (including specific modes from WavePorts) are properly identified and tracked throughout the simulation workflow.

Important Files Changed

Changed Files
Filename Score Overview
CHANGELOG.md 5/5 Added changelog entry documenting the WavePort mode index bug fix in antenna metrics
tidy3d/plugins/smatrix/component_modelers/base.py 5/5 Fixed automatic mode_index handling for WavePort objects in task name generation
tidy3d/plugins/smatrix/data/terminal.py 5/5 Updated type signature from str to NetworkIndex for better type safety and API clarity
tidy3d/plugins/smatrix/analysis/antenna.py 4/5 Core bug fix for WavePort mode handling in antenna metrics calculations
tests/test_plugins/smatrix/terminal_component_modeler_def.py 5/5 Increased simulation domain size for coaxial structures to improve test reliability
tests/test_plugins/smatrix/test_terminal_component_modeler.py 5/5 Added parameterized testing for both lumped and wave port antenna calculations

Confidence score: 4/5

  • This PR addresses a specific bug with clear fixes and should be safe to merge with minimal risk
  • Score reflects solid implementation with proper type improvements and comprehensive testing coverage
  • The antenna analysis file requires attention due to the complexity of the mode handling logic changes

Sequence Diagram

sequenceDiagram
    participant User
    participant TerminalComponentModelerData
    participant AntennaAnalysis as antenna.get_antenna_metrics_data
    participant Modeler as TerminalComponentModeler
    participant WavePort
    participant DirectivityMonitor
    participant SimulationData

    User->>TerminalComponentModelerData: get_antenna_metrics_data(port_amplitudes, monitor_name)
    TerminalComponentModelerData->>AntennaAnalysis: get_antenna_metrics_data(self, port_amplitudes, monitor_name)
    
    alt port_amplitudes is None
        AntennaAnalysis->>Modeler: get first port from modeler.ports[0]
        AntennaAnalysis->>WavePort: check if isinstance(first_port, WavePort)
        WavePort-->>AntennaAnalysis: return True/False
        alt is WavePort
            AntennaAnalysis->>WavePort: get mode_index from first_port.mode_index
            WavePort-->>AntennaAnalysis: return mode_index
        else not WavePort
            AntennaAnalysis->>AntennaAnalysis: set mode_index = None
        end
        AntennaAnalysis->>Modeler: network_index(first_port, mode_index)
        Modeler-->>AntennaAnalysis: return NetworkIndex
        AntennaAnalysis->>AntennaAnalysis: create port_amplitudes dict with NetworkIndex: None
    end

    AntennaAnalysis->>AntennaAnalysis: create port_dict from port_amplitudes keys
    loop for each key in port_amplitudes
        AntennaAnalysis->>Modeler: get port from network_dict[key]
        Modeler-->>AntennaAnalysis: return (port, mode_index)
        AntennaAnalysis->>AntennaAnalysis: map port to amplitude in port_dict
    end

    alt monitor_name is None
        AntennaAnalysis->>Modeler: get first radiation monitor from modeler.radiation_monitors[0]
    else monitor_name provided
        AntennaAnalysis->>Modeler: get_radiation_monitor_by_name(monitor_name)
    end
    Modeler-->>AntennaAnalysis: return DirectivityMonitor

    AntennaAnalysis->>TerminalComponentModelerData: get port_power_wave_matrices
    TerminalComponentModelerData-->>AntennaAnalysis: return (a_matrix, b_matrix)

    loop for each port, amplitude in port_dict
        AntennaAnalysis->>Modeler: network_index(port)
        Modeler-->>AntennaAnalysis: return port_in_index
        AntennaAnalysis->>Modeler: get mode_index from network_dict[port_in_index]
        Modeler-->>AntennaAnalysis: return (_, mode_index)
        
        alt amplitude is not None and not close to zero
            AntennaAnalysis->>Modeler: get_task_name(port, mode_index)
            Modeler-->>AntennaAnalysis: return task_name
            AntennaAnalysis->>TerminalComponentModelerData: get simulation data from data[task_name]
            TerminalComponentModelerData-->>AntennaAnalysis: return SimulationData
            
            AntennaAnalysis->>SimulationData: get monitor data [rad_mon.name]
            SimulationData-->>AntennaAnalysis: return DirectivityData
            
            AntennaAnalysis->>TerminalComponentModelerData: _monitor_data_at_port_amplitude(port, monitor_name, amplitude, a_raw)
            TerminalComponentModelerData-->>AntennaAnalysis: return scaled_directivity_data
            
            AntennaAnalysis->>AntennaAnalysis: combine scaled_directivity_data with existing combined_directivity_data
            AntennaAnalysis->>AntennaAnalysis: accumulate scaled power wave amplitudes (a_sum, b_sum)
        end
    end

    AntennaAnalysis->>AntennaAnalysis: compute power_incident and power_reflected from accumulated amplitudes
    AntennaAnalysis->>AntennaAnalysis: create AntennaMetricsData.from_directivity_data(combined_data, power_incident, power_reflected)
    AntennaAnalysis-->>TerminalComponentModelerData: return AntennaMetricsData
    TerminalComponentModelerData-->>User: return AntennaMetricsData
Loading

@dmarek-flex dmarek-flex self-assigned this Oct 9, 2025
@dmarek-flex dmarek-flex added RF rc3 3rd pre-release labels Oct 9, 2025
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

github-actions bot commented Oct 9, 2025

Diff Coverage

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

  • tidy3d/plugins/smatrix/analysis/antenna.py (100%)
  • tidy3d/plugins/smatrix/component_modelers/base.py (100%)
  • tidy3d/plugins/smatrix/data/terminal.py (100%)

Summary

  • Total: 11 lines
  • Missing: 0 lines
  • Coverage: 100%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rc3 3rd pre-release RF

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant