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

Use simulation reduction automatically only in case of custom mediums #1371

Merged
merged 3 commits into from
Jan 11, 2024

Conversation

dbochkov-flexcompute
Copy link
Contributor

  • shrink bounds when performing grid.discretize_inds on new region to avoid including possible spurious pixel
  • switch simulation reduction in mode solver to be automatic only for simulations with custom medium + display warning if that happens

if reduce_simulation == "auto":
sim = mode_solver.simulation
contains_custom = any(isinstance(s.medium, AbstractCustomMedium) for s in sim.structures)
contains_custom = contains_custom or isinstance(sim.medium, AbstractCustomMedium)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We do something like that in a bunch of places but yeah it seems like there's still no in-built way to get "all media, including the simulation one" and "all structures, including the simulation as a structure"...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I guess I could've just used Simulation.mediums

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah yeah so there is something like that!

" Setting `reduce_simulation=True` will force simulation reduction in all cases and"
" silence this warning."
)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it looks good, probably we should update the notebooks then to avoid the warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

was there any other besides BullseyeCavityPSO?

Copy link
Collaborator

Choose a reason for hiding this comment

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

BullseyeCavityPSO will not have a warning anymore as it doesn't use custom medium, I was thinking of the heat notebooks actually.

tidy3d/plugins/mode/mode_solver.py Outdated Show resolved Hide resolved
@dbochkov-flexcompute
Copy link
Contributor Author

addressed the comments, also updated heat notebooks flexcompute/tidy3d-notebooks#24

note, there is a change in web api container.py, I noticed that it was causing spurious errors due to pydantic trying to initialize the field Batch.simulations with every possible simulation class

@momchil-flex momchil-flex merged commit 0683c5c into pre/2.6 Jan 11, 2024
16 checks passed
@momchil-flex momchil-flex deleted the daniil/fix-reduced-mode-solver branch January 11, 2024 18:13
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.

2 participants