Skip to content

Fix EME port modes symmetry expansion #2605

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

caseyflex
Copy link
Contributor

@caseyflex caseyflex commented Jun 25, 2025

This fixes two related issues:

Copy link
Collaborator

@lucas-flexcompute lucas-flexcompute left a comment

Choose a reason for hiding this comment

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

My only comment is to maybe define port_modes_raw as an internal property _port_modes_raw if we don't want the user poking there. Unless this is handled similarly for other monitors, then just keep it consistent…

Copy link
Contributor

Diff Coverage

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

  • tidy3d/components/eme/data/sim_data.py (100%)

Summary

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

@caseyflex
Copy link
Contributor Author

My only comment is to maybe define port_modes_raw as an internal property _port_modes_raw if we don't want the user poking there. Unless this is handled similarly for other monitors, then just keep it consistent…

Thanks for the comment. In ModeSimulation we also use modes_raw and modes as the symmetry expanded version. I think there is an issue with the way pydantic handles fields starting with an underscore (it doesn't treat them as actual fields).

@caseyflex caseyflex force-pushed the casey/emesymmetry branch from 2c2309e to a9545d4 Compare June 26, 2025 18:13
@caseyflex caseyflex force-pushed the casey/emesymmetry branch from a9545d4 to 0754a1c Compare June 26, 2025 18:14
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