Conversation
|
Ok think this should be ready now, are you ok to give this another pass when next at your desk? @johnbcoughlin 🙏🏻 |
adept/normalization.py
Outdated
| tau: UREG.Quantity | ||
|
|
||
| # Electron density (may differ from normalization density n0) | ||
| ne: UREG.Quantity | None = None |
There was a problem hiding this comment.
This isn't suitable to put on the Normalization object in my opinion.
There was a problem hiding this comment.
Ok I tried dependency inject it into initialize distribution function instead. I'm a bit meh about the whole set up here for now, but I think it'll do.
|
|
||
| T0 = UREG.Quantity(cfg_units["reference electron temperature"]) | ||
| ne = UREG.Quantity(cfg_units["reference electron density"]).to("1/cc") | ||
| n0 = UREG.Quantity("9.0663e21/cm^3") |
There was a problem hiding this comment.
I don't really understand why the normalization object needs to store both ne and n0. It should only need one number density, which is the density in 1/cc that "1" means in a simulation. If the actual ne in simulation is different than that, then that's a property of the simulation / initial condition, not the normalization.
| ne: UREG.Quantity | None = None | ||
| # Coulomb logarithms | ||
| logLambda_ei_val: float | None = None | ||
| logLambda_ee_val: float | None = None |
There was a problem hiding this comment.
can these be methods like logLambda_ee below, rather than fields?
There was a problem hiding this comment.
Its a bit tricky because it depends on Z and ni and they might be spatially dependent, I'm not completely clear about your architectural decision and what the advantages of using a method are, happy for you to explain further.
adept/vfp1d/base.py
Outdated
| n0 = u.Quantity("9.0663e21/cm^3") | ||
| ion_species = self.cfg["units"]["Ion"] | ||
| ne = norm.ne | ||
| Te = norm.T0.to("eV") |
There was a problem hiding this comment.
Would be more readable to convert to eV and extract the magnitude on the same line, or name this Te_eV.
| vth = norm.v0.to("m/s") | ||
| beta = 1.0 / norm.speed_of_light_norm() | ||
| # Elementary charge in Gaussian CGS units (Franklin) | ||
| e_gauss = UREG.Quantity(4.803204712570263e-10, "Fr") |
There was a problem hiding this comment.
this isn't necessary, you can just use UREG.e and it will automatically do the conversions for you.
There was a problem hiding this comment.
This doesn't seem to work as conversion beween gaussian and SI doesn't work with the ureg/jpu's model it seems. I don't want to rewrite the fomrula in SI as more ugly.
2 participants
I tried to mirror what you guys were doing in Vlasov1D and move towards dependency inject, just a quick pass to check that its in keeping with where you're going and I'm not doing anything silly, cheers