-
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
Charge solver #2077
Charge solver #2077
Conversation
@tylerflex do tell me if you'd prefer someone else to review this. |
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.
Did a first pass through it, want to look into it in a bit more detail afterwards possibly as we chat.
I guess most of my comments are actually semi-philosophical implementation questions that are kind of open and a bit in the air anyways - so quite interested to hear everyone's related future vision-thoughts on integration considerations!
But looks incredible, such a great effort and really well done - congrats Marc! Really cool that it's coming together quite nicely 💪
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.
Left some initial comments and questions. Definitely, will give a couple more passes. Major comments so far:
- It seems currently we have only one monitor type for charge simulations that returns all data. This seems to contradict the approach we took with fdtd simulations, where we have separate monitors or each type of data, like
FieldMonitor
,FluxMonitor
,PermittivityMonitor
, etc. This also allows to record and transfer only data which is needed. For example, if I am solving charge equations to compute waveguide index change due to free carriers I would like to download only free carrier concentrations. If my waveguide index change is due to electric field, then I would only want to download that (or potential). Would there be any issues with following that approach? Meaning having something likePotentialMonitor
,FreeCarriersMonitor
,CapacitanceMonitor
,CurrentMonitor
, etc? - From what I understand currently setting
electric_spec
toSemiConductorSpec
means that the material is silicon. Would it make sense to generalizeSemiConductorSpec
and make it have explicit parameters that go into material model inside the charge solver? Like mobilities, effective mass, life time, bandgap, etc
No problem at all with that. I just thought it'd be annoying to have to rerun a simulation if, say, you wanted to retrieve what the electric potential looks like. Currently modifying this behavior along the lines you suggested above
Yes, 100%. Right now, thought, what I'm not sure about is what to do with the model parameters. Currently I have these for Si:
These models are, in general, dependent on the Material. So my initial thought was to add material to the class:
An issue with this approach is that I'd need to expose All this, highlights another task that I have in my pipeline. I.e., currently, all these material models are active by default. I have a task so that users can deactivate these models at will. |
What is SemiconductorDatabase? Just to think if as a user I'd want to edit it. Is it like the internal mesh model? |
Also thought Chapter 11.3 of the ngspice documentation might be of interest in terms of inspired data types and simultion configuration parameters https://ngspice.sourceforge.io/docs/ngspice-manual.pdf . If you see some modulator design papers you'll see everyone ultimately is building equivalent circuit models. See this pretty decent paper if you're interested to have in mind ultimately how some of this simulation data could be transformed |
Basically it'd store all model parameters associated with the material. |
Thanks @daquinteroflex, I'll be reading through this! |
would it make sense to do something like:
then to define a material one would do
so that depending on material different models can swapped, added, removed, etc? |
While I like the solution you propose, this implies that the user must know what values to add depending on the material. So far, I'm coding this with default values for Silicon but maybe we should consider having a database for these models for other semiconductors. |
So just to add, this is kind of a complicated topic. For example, say this type of semiconductor spec can be defined in other tools - but it can be extremely difficult to know how to define these models. However, advanced users do need to have an interface for these material definitions and it'd be expected that those users would know how to define this. However, we need to make sure we can abstract on top of this and provide the most common easy-to-use models for the most common materials that can accurately represent the device physics. There are multiple research groups that do their own photonic device fabrication in house, and they have different recipes in the cleanroom that can change like crystal alginments(polarization) etc. properties that directly affects RF electrostatic perturbation, device purity, etc. I think ultimately we want to give them tools to try to match simulations to their fabrication by enabling them to access the lower level fundamental material representations (on the lines maybe of It sounds like something on the lines of this |
7c2399e
to
73ef52f
Compare
Thanks @daquinteroflex @dbochkov-flexcompute @momchil-flex for all the comments. This has, consequently, influenced the backend PR, which has been updated accordingly. Any more suggestions/comments are welcome! |
9aa82c9
to
51271b3
Compare
bae9dc8
to
812d67c
Compare
75b87b0
to
947474b
Compare
08b7318
to
e1bd95e
Compare
d9cbbef
to
fb3e409
Compare
fb3e409
to
85b6546
Compare
85b6546
to
9eb25bf
Compare
9eb25bf
to
5e6ddc3
Compare
description="A current source", | ||
units=CURRENT_DENSITY, | ||
) | ||
# TODO translation between currentsource amps and currentdensity, why not amps 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.
Also this @marc-flex
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.
I'd just remove the TODO and maybe open an issue to support the integral quantity in the future?
The main issue here is that, as far as I know, there isn't an easy way to get the area of a contact in the heat solver before you start simulating. Even if this is not currently supported, it shouldn't the end of the world to implement it.
I guess the second issue is a mathematical one. If one imposes an integral quantity as BC, you're forcing the solution to have uniform flux through a surface which may not make much sense depending on the geometry.
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.
This seems like an important functionality also for RF as per Damian's comments. Yeah also in the future we might want to specify a varying current intensity for a given area so probably need to extend this type of definition in a slightly different way eventually. (or compute the total effective current for a given area etc. as observed for DC frequencies)
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.
In addition to comments below, probably my major concern is about that all charge medium models have default values for Si. I think this can be extremely dangerous. For example, if I want to define a SemicondutorMedium
specification for, say, Ge, then I would need to remember all the parameters and models that I need to change and which would be otherwise set to Si values. I think not having any default parameters and let it just error if not provided is safer. While this example specification for Si with charge models and parameters could be added to materials_library
dictionary. Maybe as _multiphysicsSi
to highlight it's a temporary convenience until materials_library
is properly generalized to multiphysics mediums?
Yep, agree, am implementing the silicon library in #2180 (comment) EDIT: Sorted now. |
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.
just trying to catch anything small. I noticed that many new introduced fields don't have units=...
specified, I commented under one, but saw many others
tidy3d/components/tcad/doping.py
Outdated
"""The sigma parameter of the pseudo-gaussian""" | ||
return np.sqrt(-self.width * self.width / 2 / np.log(self.ref_con / self.concentration)) | ||
|
||
def get_contrib(self, coords: dict, meshgrid: bool = True): |
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.
is this a user-facing function? if not, can rename it as _get_contrib
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.
It could be used by users to validate their own dopings. I guess that's why I made a test specific for this function. Though I doubt they'll ever use it? So happy to change the name
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.
I find it useful to not expose such functions to the user by adding a _
, so that if I need to change something later I don't need to worry about backward compatibility
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.
Sounds good. I'll change it to have _
ed78fc8
to
9a371e7
Compare
708f8e3
to
a7c3c62
Compare
Co-authored-by: Dario Quintero <dario@flexcompute.com>
a7c3c62
to
430b5e3
Compare
Introducing Charge simulations with Devsim.
DevsimConvergenceSettings
has been introduced in which users can specify convergence criteria an other Devsim related settings.ChargeSimulationMonitor
, has been created that deals with all charge data (C-V curve, charge, doping and potential)HeatChargeSimulation
has been adapted so that it copes with the new simulation typeDEVSIM
. This includes the plotting functionIVCurveDataArray
andCapacianceDataArray
have been created to deal with the I-V and C-V curves and its dimensions in a consistent manner@momchil-flex @dbochkov-flexcompute for your reference