-
Notifications
You must be signed in to change notification settings - Fork 52
Warn when mode solver pml covers a significant portion of the mode plane #2320
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
Conversation
f99cb69
to
babc9a1
Compare
@momchil-flex fyi. You mentioned changing the plotting for mode monitors and mode sources to show the mode pml. I didn't include this here because I wasn't sure about it -- it would clutter the plotting, and maybe it's not that critical most of the time? But we could consider including that or unifying the plotting between the |
cea0bf4
to
3446b3c
Compare
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 @caseyflex looks good to me! We really need a config though for these globals... 😄
3446b3c
to
7d62a18
Compare
@e-g-melo can we merge this? |
7d62a18
to
7fbee9e
Compare
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.
Hi @caseyflex! Thanks for this work!
I am ok with the changes to plotting.
After trying it, I suggest reducing that threshold to 50% of the mode plane.
Sorry, one thing this is making me think about is that e.g. in RF CPW simulations, the mesh could be much much finer around the waveguide than at the outer edges of the mode plane. So while the physical area of the PML may appear large, there is no problem because the majority of the grid cells are outside of PML. I feel like generally checking on the grid points level rather than physical area might be better? |
7fbee9e
to
263bce8
Compare
@momchil-flex I reworked it so the warning is now based on num_cells (although the area functions are still available if needed) |
263bce8
to
fdf4de7
Compare
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.
Looks good!
Partially addresses this issue:
#2277
Adds a warning when the PML covers a significant portion of the mode plane, and makes it easy to get the mode PML thickness (e.g. for GUI plotting).
Doesn't change the mode PML behavior, API, or plotting.