Skip to content
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

port_symmetries on gtidy3d.write_sparameters seem broken #1681

Closed
jan-david-fischbach opened this issue May 21, 2023 · 9 comments · Fixed by #1683 or #1692
Closed

port_symmetries on gtidy3d.write_sparameters seem broken #1681

jan-david-fischbach opened this issue May 21, 2023 · 9 comments · Fixed by #1683 or #1692
Labels
bug Something isn't working

Comments

@jan-david-fischbach
Copy link
Collaborator

Describe the bug
When using port symmetries it seems to execute the wrong simulations and not copy over the s-parameters correctly

To Reproduce

c = gf.components.mmi2x2()
symmetry = {
  "o1": {
    "s11": ["s22","s33","s44"], "s21": ["s12","s34","s43"], "s31": ["s13","s24","s42"], "s41": ["s14","s23","s32"],
  }
}
df = gt.write_sparameters(c, overwrite=True, port_symmetries=symmetry, is_3d=False, verbose=True)

Expected behavior
Execute a single simulation (not three) and copy the s-parameters as advised by the dict.

Suggested fix
I will open a PR

@jan-david-fischbach jan-david-fischbach added the bug Something isn't working label May 21, 2023
@jan-david-fischbach
Copy link
Collaborator Author

Additionally it seems to break with the old s-parameter notation. So either we should autoconvert from the old notation to the new one (assuming fundamental mode), or we should fix the docs...

jan-david-fischbach added a commit to jan-david-fischbach/gdsfactory that referenced this issue May 21, 2023
@joamatab joamatab reopened this May 22, 2023
@joamatab
Copy link
Contributor

joamatab commented May 22, 2023

it seems still has issues

image

@lucas-flexcompute

@lucas-flexcompute
Copy link
Contributor

The problem seems to be the name of the ports. The names coming from the component are 'o1', 'o2', but the symmetry definition uses another naming scheme, so this if clause is never true.

@jan-david-fischbach
Copy link
Collaborator Author

I had been "fixing" the issue according to the [doc string] (

). There the port names are used as keys. The format specifying the excite port actually makes sense to me, so we would have to adjust the symmetry definitions. What do you think @joamatab?

@jan-david-fischbach
Copy link
Collaborator Author

jan-david-fischbach commented May 23, 2023

  • And we should add some tests to avoid breakage in the future :)

@joamatab
Copy link
Contributor

we have tests for the tidy3d plugin and are failing at the moment

@jan-david-fischbach
Copy link
Collaborator Author

Interesting... I'll have to check why the tests passed on my machine...

@jan-david-fischbach
Copy link
Collaborator Author

Regarding the top level keys of the symmetry dicts: do we stick to using the excitation port?

@joamatab
Copy link
Contributor

joamatab commented May 23, 2023

when you work on a fork that has no credentials for tidy3d it skips those tests

I think i found a fix, see PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants