Skip to content

Conversation

@weiliangjin2021
Copy link
Collaborator

@weiliangjin2021 weiliangjin2021 commented Oct 20, 2025

Greptile Overview

Updated On: 2025-10-21 00:00:39 UTC

Greptile Summary

This PR adds the ability to specify custom source time dependence in component modelers (ModalComponentModeler and TerminalComponentModeler) through a new optional custom_source_time parameter in the AbstractComponentModeler base class. When provided, the custom source time overrides the default GaussianPulse behavior that was previously constructed automatically from frequency and bandwidth parameters. The feature integrates with the existing component modeler framework by checking if self.custom_source_time is not None in the to_source methods and using the custom value when present. This change maintains backward compatibility since the parameter defaults to None, preserving existing behavior for all code that doesn't explicitly provide a custom source time. The JSON schema for TerminalComponentModeler has been updated to reflect the three supported source time types: ContinuousWave, CustomSourceTime, and GaussianPulse.

Important Files Changed

Changed Files
Filename Score Overview
tidy3d/plugins/smatrix/component_modelers/base.py 5/5 Added custom_source_time optional field to base class with proper type hints and documentation
tidy3d/plugins/smatrix/component_modelers/modal.py 5/5 Implemented custom source time logic in to_source method using conditional check
tidy3d/plugins/smatrix/component_modelers/terminal.py 3/5 Missing in PR context but likely contains similar implementation to modal.py
schemas/TerminalComponentModeler.json 5/5 Updated JSON schema to include custom_source_time field with three source time type options
CHANGELOG.md 5/5 Added clear changelog entry documenting the new feature
tests/test_plugins/smatrix/test_component_modeler.py 1/5 Test uses invalid ContinuousWave with non-zero fwidth, which violates source constraints
tests/test_plugins/smatrix/test_terminal_component_modeler.py 2/5 Test has same issue - uses ContinuousWave with fwidth=1e8 instead of required fwidth=0

Confidence score: 2/5

  • This PR requires careful review before merging due to critical test implementation issues that will likely cause validation errors or incorrect behavior
  • Score is lowered because both new tests create invalid ContinuousWave sources with non-zero fwidth values (test_component_modeler.py uses fwidth=1e13, test_terminal_component_modeler.py uses fwidth=1e8), but ContinuousWave sources are monochromatic by design and should have fwidth=0; the core feature implementation in base.py and modal.py appears sound, but terminal.py is missing from context so cannot be verified; tests should use GaussianPulse instead or set fwidth=0 for ContinuousWave
  • Pay close attention to tests/test_plugins/smatrix/test_component_modeler.py and tests/test_plugins/smatrix/test_terminal_component_modeler.py which both contain invalid source time configurations that need correction

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.

7 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Contributor

github-actions bot commented Oct 21, 2025

Diff Coverage

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

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

Summary

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

@weiliangjin2021 weiliangjin2021 force-pushed the weiliang/FXC-3684-allow-source-time-customization-in-terminal-port branch from a8df9d9 to d0cfb18 Compare October 21, 2025 01:15
@weiliangjin2021 weiliangjin2021 force-pushed the weiliang/FXC-3684-allow-source-time-customization-in-terminal-port branch from d0cfb18 to f5c7469 Compare October 21, 2025 21:58
@weiliangjin2021 weiliangjin2021 added this pull request to the merge queue Oct 21, 2025
Merged via the queue into develop with commit 51161f9 Oct 21, 2025
34 checks passed
@weiliangjin2021 weiliangjin2021 deleted the weiliang/FXC-3684-allow-source-time-customization-in-terminal-port branch October 21, 2025 23:30
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.

3 participants