-
Notifications
You must be signed in to change notification settings - Fork 50
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
add WavePorts #1741
add WavePorts #1741
Conversation
8cfd8b6
to
4760d74
Compare
@weiliangjin2021 @QimingFlex Still a bit rough at the moment in terms of docstrings and other details. But if you want to take a look over the next week while I am validating the accuracy, I am ready for feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1st pass through the code. I'll definitely need to take several more rounds to have a better understanding. Very cool to see all components stick together! One main question is if we should support multiple ways of computing impedance: using any two quantities of V, I, P.
Yeah, it is all hidden away in the |
612f551
to
c49602c
Compare
Might help for your review to check out this notebook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went through another pass. All look quite nice to me! I'm still wondering if we have a way to compute currents that doesn't need contour path integration. One way that I can think of is as follows (but don't know if it works well):
- On the modal plane (let's assume it to be xy-plane), compute J at every grid point through J = Curl H. Next we integrate Jz over the entire modal plane. During the integration, we integrate only positive or negatie Jz, as otherwise the current cancels out. So it's int dS Heaviside(J_Z) J_z. Does it agree with the contour path approach for TEM mode?
Some minor comments:
- It'll be very helpful to also visualize voltage/current integration path in the plot.
- For voltage integration path, in many cases, it is an straight line. When it's axis-aligned, it'll be more convenient to allow the definition through 2 endpoints?
That could work, and it seems like a discrete version of Stoke's theorem, so should be equivalent. It is also probably an easier way to implement the computation on the Yee grid when the conductors are complicated shapes. I don't think that I could use that method for computing currents at the ports though, because we do not know whether the current is flowing into or out of the port before computing. Could be used for compute Z0 when we can assume the propagation direction. However, since it seems possible to apply this to more complex shapes more easily, I think I will try. We could still make the user define a path around the conductor of interest. Instead of computing the line integral though, we compute the current inside the contour using curl H.
Yes, good idea! I'll experiment.
So do you mean an option to create path integrals given P1(x1,y1,z1) and P2(x2,y2,z2) and just ensure that that it is axis aligned with a validator? If so I can do that! |
Hopefully we can also support axis-unaligned straight path. But you mentioned there is complication for that |
Agreed, at least we can easily swap out implementations for voltage and current integrals easily. But that is an improvement that is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @dmarek-flex . Looks overall good to me. Although I can't follow many of the specific details.
5239d79
to
287892f
Compare
cc4fb27
to
f6c0c40
Compare
@@ -73,18 +73,15 @@ def _plot(*args, **kwargs) -> Ax: | |||
""" plot parameters """ | |||
|
|||
|
|||
class PlotParams(Tidy3dBaseModel): | |||
"""Stores plotting parameters / specifications for a given model.""" | |||
class AbstractPlotParams(Tidy3dBaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tylerflex Might want to check my changes here which I used for plotting the path integrals
@@ -684,6 +684,29 @@ def parse_xyz_kwargs(**xyz) -> Tuple[Axis, float]: | |||
axis = "xyz".index(axis_label) | |||
return axis, position | |||
|
|||
@staticmethod | |||
def parse_two_xyz_kwargs(**xyz) -> List[Tuple[Axis, float]]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tylerflex also here which might affect others
f6c0c40
to
e2915f8
Compare
added path integral plotting functionality added convenience function for defining voltage path integral from end points
e2915f8
to
949c712
Compare
@weiliangjin2021 I am ready to merge this as soon as you confirm. Tests are passing after rebase and my testing notebook is giving good results. I just changed the CHANGELOG to reflect the changes in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please go ahead
add a new type of port to the TerminalComponentModeler. WavePorts are the optimal port for exciting transmission lines, and they function like a hybrid between LumpedPorts and the modal-based Port.
Still a work in progress.
Here are some thoughts:
ModeSolverMonitor
s along with the tidy3d simulations. However, some users might want to know the impedances beforehand or inspect modes before launching the simulations.TerminalComponentModeler
are based on pairs of terminals ( a positive and negative connection or node in the circuit).LumpedPort
s andWavePort
are then bothTerminalPort
sname
s and possiblyattrs
toDataArray
s that I am creating. Maybe I should put all of theseDataArray
s into the microwave plugin or the Tidy3D files.