Skip to content

Conversation

@dbochkov-flexcompute
Copy link
Contributor

@dbochkov-flexcompute dbochkov-flexcompute commented Oct 17, 2025

Here's a working version for automatic generation of directivity monitors in terminal component modelers. It creates a directivity monitor 2 cells away from pml/domain boundaries, and also checks that it is at least 2 cells away from structures and lumped elements. Re-runs of a couple of cases using this feature:

A bit unsure about the API. Currently, it works as

tcm = TerminalComponentModeler(..., radiation_monitors="auto")

but maybe a better way is to introduce some sort of AutoDirectivityMonitor

tcm = TerminalComponentModeler(..., radiation_monitors=[AutoDirectivityMonitor()])

where it is possible to adjust parameters AutoDirectivityMonitor(freqs=..., num_theta_points=..., num_phi_points=..., name=..., buffer=...)

what do you think?

Greptile Overview

Updated On: 2025-10-17 04:49:09 UTC

Greptile Summary

This PR introduces automatic directivity monitor generation for antenna calculations in terminal component modelers. The key feature allows users to specify radiation_monitors="auto" in TerminalComponentModeler, which automatically creates and positions a DirectivityMonitor for far-field radiation pattern analysis.

The implementation intelligently positions the monitor 2 grid cells away from PML/domain boundaries while ensuring adequate clearance from structures and lumped elements. The auto-generated monitor samples the full sphere with configurable angular resolution (100 theta points, 200 phi points by default) and includes comprehensive validation to prevent placement issues.

The change maintains backward compatibility - users can still provide custom DirectivityMonitor objects as before, or use the new "auto" convenience feature. The implementation uses proper separation of concerns with dedicated methods for monitor generation, validation, and simulation finalization. This feature significantly improves the usability of antenna modeling workflows by automating the complex geometry considerations typically required for proper monitor placement.

Important Files Changed

Changed Files
Filename Score Overview
tidy3d/plugins/smatrix/component_modelers/terminal.py 4/5 Core implementation of automatic DirectivityMonitor generation with validation and placement logic
tests/test_plugins/smatrix/test_terminal_component_modeler.py 5/5 Added comprehensive tests for automatic radiation monitor creation and error handling
tests/test_plugins/smatrix/terminal_component_modeler_def.py 5/5 Added patch antenna test case functions to support automatic directivity monitor testing
tidy3d/plugins/smatrix/analysis/antenna.py 4/5 Updated to use finalized radiation monitors instead of raw monitors for consistency

Confidence score: 4/5

  • This PR introduces a valuable feature that improves antenna modeling usability with well-structured implementation and comprehensive testing
  • Score reflects one minor typo in error message ("sizet" instead of "size") and potential for future API considerations around customizable parameters
  • The core implementation follows established patterns and includes proper validation, error handling, and backward compatibility

Sequence Diagram

sequenceDiagram
    participant User
    participant TerminalComponentModeler
    participant Simulation
    participant RadiationMonitorGenerator
    participant ValidationEngine

    User->>TerminalComponentModeler: "Create modeler with radiation_monitors='auto'"
    TerminalComponentModeler->>TerminalComponentModeler: "_finalized_radiation_monitors property accessed"
    TerminalComponentModeler->>RadiationMonitorGenerator: "_generate_radiation_monitor()"
    RadiationMonitorGenerator->>Simulation: "Get PML thicknesses from num_pml_layers"
    RadiationMonitorGenerator->>Simulation: "Get grid boundaries"
    RadiationMonitorGenerator->>RadiationMonitorGenerator: "Calculate monitor span with AUTO_RADIATION_MONITOR_BUFFER"
    RadiationMonitorGenerator->>RadiationMonitorGenerator: "Create Box.from_bounds()"
    RadiationMonitorGenerator->>RadiationMonitorGenerator: "Generate theta and phi arrays"
    RadiationMonitorGenerator->>RadiationMonitorGenerator: "Create DirectivityMonitor"
    RadiationMonitorGenerator->>ValidationEngine: "_validate_radiation_monitor_buffer()"
    ValidationEngine->>Simulation: "Get finalized structures"
    ValidationEngine->>ValidationEngine: "Calculate union of structure bounds"
    ValidationEngine->>ValidationEngine: "Check buffer distances"
    alt Buffer validation passes
        ValidationEngine-->>RadiationMonitorGenerator: "Validation successful"
        RadiationMonitorGenerator-->>TerminalComponentModeler: "Return DirectivityMonitor"
    else Buffer validation fails
        ValidationEngine->>ValidationEngine: "Raise ValueError"
        ValidationEngine-->>User: "ValueError: Monitor too close to structures"
    end
    TerminalComponentModeler->>TerminalComponentModeler: "Add monitor to base_sim"
    TerminalComponentModeler-->>User: "Return modeler with auto radiation monitor"
Loading

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.

4 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2025

Diff Coverage

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

  • tidy3d/components/monitor.py (100%)
  • tidy3d/plugins/smatrix/analysis/antenna.py (100%)
  • tidy3d/plugins/smatrix/component_modelers/terminal.py (97.9%): Missing lines 645,688

Summary

  • Total: 99 lines
  • Missing: 2 lines
  • Coverage: 97%

tidy3d/plugins/smatrix/component_modelers/terminal.py

Lines 641-649

  641         lumped_elements = simulation.lumped_elements
  642 
  643         # If no structures or lumped elements, validation passes
  644         if not structures and not lumped_elements:
! 645             return
  646 
  647         # Calculate union of bounding boxes for all structures and lumped elements
  648         all_geoms = []

Lines 684-692

  684 
  685                 # Check plus side: union should be at least BUFFER cells away from monitor end
  686                 buffer_plus = mnt_end - union_end
  687                 if buffer_plus < buffer:
! 688                     raise ValueError(
  689                         f"Automatically generated radiation monitor is too close to structures on the positive {axis_name} side. "
  690                         f"Buffer: {buffer_plus} cells, required: {buffer} cells. "
  691                         f"Please increase simulation domain size."
  692                     )

@dmarek-flex
Copy link
Contributor

where it is possible to adjust parameters AutoDirectivityMonitor(freqs=..., num_theta_points=..., num_phi_points=..., name=..., buffer=...)

what do you think?

Yea I think users might want to change a few of these parameters (I am also curious if the size of the buffer will affect the far field results).

I got used to the Spec pattern, like AutoGridSpec, which is a description of how to build the object. So maybe we can support the current manual approach and in addition the user can supply a RadiationSpec or perhaps DirectivitySpec and then we can support multiple different types of specs in the future for different levels of automation.

@dmarek-flex
Copy link
Contributor

And maybe we should keep the list/tuple pattern as you have, for computational efficiency it is nice to be able to make different monitors with different frequency/spatial sampling.

@dmarek-flex
Copy link
Contributor

Basically though I think this PR addresses the issue! Thanks for the quick turn around.

Copy link
Contributor

@George-Guryev-flxcmp George-Guryev-flxcmp left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just have a few minor comments regarding naming conventions and docstrings.

Copy link
Contributor

@dmarek-flex dmarek-flex left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks again! Just a few minor comments

@dbochkov-flexcompute dbochkov-flexcompute force-pushed the daniil/FXC-3577-Improve-usability-of-far-field-antenna-calculations branch 3 times, most recently from fa07f54 to ea4b48b Compare October 28, 2025 23:23
@dbochkov-flexcompute dbochkov-flexcompute force-pushed the daniil/FXC-3577-Improve-usability-of-far-field-antenna-calculations branch from ea4b48b to b5ed563 Compare October 29, 2025 00:28
@dbochkov-flexcompute dbochkov-flexcompute added this pull request to the merge queue Oct 29, 2025
Merged via the queue into develop with commit cb3e2d0 Oct 29, 2025
25 checks passed
@dbochkov-flexcompute dbochkov-flexcompute deleted the daniil/FXC-3577-Improve-usability-of-far-field-antenna-calculations branch October 29, 2025 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants