-
Notifications
You must be signed in to change notification settings - Fork 52
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
Structure priority option in structure overlapping region #2336
base: develop
Are you sure you want to change the base?
Conversation
Haven't had time to review properly yet but a quick thought: Maybe it would make sense if we introduced a numbers-based priority to structures in general? By default they'd all have the same, and then priority is decided on ordering, but giving a larger number would mean higher priority. This is a pretty common approach I think, for example matplotlib does it for |
That's actually my initial plan. But the concern is that this feature will barely be used, and people will only use the automated priority approach. @tomflexcompute also brought up this when he is using a certain photonic simulation software. |
For backward-compatibility, we cannot by default assign higher priority to metal. So we will still need an additional field like the one added in this PR that sets the automation behavior: by structure list order or material property. To that regard, I think we can just have this additional field for automation for now, and adds the manual priority number when requested in the future. Now regardless of the approach, I think we'll still need |
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.
Yeah I'm not sure honestly, I see the advantage of the current approach in that it's easier for the user to set, but it does feel like we'll just keep running into the need to have a priority setting in the structures. It seems like it could already make at least the internal code a bit better, and then if we have it internally, we might expose it anyway.
If we were to do that, how might the metallic
option combine with it?
tidy3d/components/scene.py
Outdated
title="Resolution Strategy In Structure Overlapping Region", | ||
description="In regions of spatial overlapping between structures, " | ||
"if ``overlapping_priority=latter``, structures defined later in the list haver higher " | ||
"priority. If ``overlapping_priority=metal``, metallic structures of higher conductivity " |
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.
By metallic
we explicitly mean PEC or SIBC but not e.g. usual dispersive metals?
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.
we don't have any special subpixel methods applied to usual dispersive metals. So we only mean "good conductor" by "metallic" here.
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.
Now it's renamed to conductor
sigmas[ind] = mat.conductivity | ||
return sigmas[0] - sigmas[1] | ||
|
||
return sorted(self.structures, key=cmp_to_key(structure_comparator)) |
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.
There was some rearrangement done for 2D media too, right? I think it was to put the equivalent volumetric structures on top. I wonder how this plays with these changes, and if it's at all possible to unify these rearrangements more - also to clarify even better to the user what exactly happens in various cases (metals + 2D media? very strange case I guess but still)
Could you give more contexts on what other occasions that requires a manual priority setting? |
What's your thought on improving the internal codes with manual priority setting? I think we'll still need the
Yeah, that's my question too. In this approach, I think the priority field in structure will be |
I think the priority should just be an |
ee0de0a
to
aeca342
Compare
@momchil-flex One question on meshing: in this part, we have both physical structures and mesh override structures. There are two directions for us to handle this:
Personally, I prefer the 2nd approach. For the 1st approach, since for now the default priority of override structures is 0, and metallic physical structures have higher priority, override structures might never take effect. |
Another question to @dmarek-flex and @caseyflex: in the |
2nd approach makes sense to me too. I think for some very esoteric situations where someone wants to have a regular structure above a meshing override one, they can just also add the regular structures in the override structures list. |
It looks to me like currently, when a Medium2D is encountered, it stays where it is in the list of structures. This choice was made to allow defining structures like in this notebook: https://docs.flexcompute.com/projects/tidy3d/en/latest/notebooks/Bandstructure.html where you place a uniform medium (which could be a Medium2D), and then put air holes into it |
Good question, current behavior of lumped elements is that they are always appended to the For general |
In
OK, we will not assign them special priority. Their priority will only be boosted, if they are 2D PEC. |
In many RF smulations, metallic structures are usually very thin: thickness much smaller than grid size. We can still accurately simulate them by applying PEC conformal meshing and SIBC techniques. However, since we slightly expand the structures in the solver (not really now, but a similar strategy), when the metallic structures are not added later compared to the touching dielectric structures in the structure list, those metallic structures will be completely missing in Yee-grid representation. It is a very common issues when people setup an RF simulation, even among our internal developers.
As discussed here, the scope of this PR:
AbstractStructure
. The default value isNone
.structure_priority_mode
toScene
. It sets priority of structures that havepriority=None
. Right now, there are two modes:equal
that setspriority=0
;conductor
that setspriority=90
for lossyMetaMedium, and 100 for PEC, and 0 for otherssorted_structures
in Scene to be applied in places wheresim.structures
was usedstructure_priority_mode
.structures = sorted_structure + sorted_mesh_override_structures