-
Notifications
You must be signed in to change notification settings - Fork 200
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
Add external particle fields ohms law hybrid #5275
Add external particle fields ohms law hybrid #5275
Conversation
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
Adding time value for particle field lookup in field substepping.
b534e43
to
5f75428
Compare
for more information, see https://pre-commit.ci
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
….com:clarkse/WarpX into add_external_particle_fields_ohms_law_hybrid
… Cylindrical and Cartesian.
for more information, see https://pre-commit.ci
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
….com:clarkse/WarpX into add_external_particle_fields_ohms_law_hybrid
for more information, see https://pre-commit.ci
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
….com:clarkse/WarpX into add_external_particle_fields_ohms_law_hybrid
…tical field values into ghost cells. The E/B fields and edge_lengths have different numbers of ghost cells. Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
…hen not enabled. Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
…ternal data loading. Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
… computed by curlA and E is computed by computing numerical derivative of seperable time component.
….com:clarkse/WarpX into add_external_particle_fields_ohms_law_hybrid
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
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've made some progress but have not fully reviewed all the changes yet. Here are some comments I've recorded so far.
...ests/ohm_solver_cylinder_compression/inputs_test_3d_ohm_solver_cylinder_compression_picmi.py
Outdated
Show resolved
Hide resolved
...ests/ohm_solver_cylinder_compression/inputs_test_3d_ohm_solver_cylinder_compression_picmi.py
Outdated
Show resolved
Hide resolved
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/ExternalVectorPotential.cpp
Outdated
Show resolved
Hide resolved
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/ExternalVectorPotential.cpp
Outdated
Show resolved
Hide resolved
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/HybridPICModel.cpp
Show resolved
Hide resolved
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/HybridPICModel.cpp
Outdated
Show resolved
Hide resolved
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/HybridPICModel.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
…ohm_solver_cylinder_compression_picmi.py Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
@roelof-groenewald thanks for the partial review. I will start making some of the non-trivial changes and rechecking the tests. Very thorough review as usual. This will make for a better PR. |
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/ExternalVectorPotential.cpp
Show resolved
Hide resolved
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
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.
Overall it looks good! I just had a final few small comments and a somewhat bigger question:
In the solve loop
But
Then in the next half of the field push,
And then at the very end of the field-solve
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/HybridPICModel.cpp
Show resolved
Hide resolved
@@ -617,6 +655,10 @@ void FiniteDifferenceSolver::HybridPICSolveECylindrical ( | |||
+ T_Algo::Dzz(Jr, coefs_z, n_coefs_z, i, j, 0, 0) - jr_val/(r*r); | |||
Er(i, j, 0) -= eta_h * nabla2Jr; | |||
} | |||
|
|||
if (include_external_fields && (rho_val >= rho_floor)) { | |||
Er(i, j, 0) -= Er_ext(i, j, 0); |
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.
Why is the external field subtracted rather than added?
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.
So it is set up this way since the external fields are what they are, they operate in the soak through limit. So because of this since it is not solving A via a Poisson solve with totally self-consistent boundaries. I initially tried to just use the numerical integration to integrate the total B field, but because if the user is not extra careful to make sure the external field is self-consistent with the boundaries this approach was taken. I am doing a subtraction at the beginning of the Hypric PIC loop to remove the external component from the total field solution, then the E and B fields are the plasma response fields only moving through the integration loop. This allows for the Hybrid loop to proceed with integrating the plasma response into B
Since the E and B fields at this point are the plasma fields in the Ohm's Law solver, however the solution is for a total field in order to get the plasma response only the E_ext needs to be subtracted from both sides of the equation.
In the vacuum region it gets a bit hairier. If the resistivity in that region is high enough or if the holmstrom flag is used then the Generalized Ohms Law asymptotes to:
This can be linearly added to the vacuum field to get the total field since the assumption is that it is a vacuum and not dense enough to shield an external field.
This resistive electric field gets integrated into the B field, then the resistivity term is dropped because of the cancellation of the drag term in the ion momentum update for the particle push.
Since the plasma response in the vacuum region is not well defined the algorithm switches to using this form of Ohm's law to have a smoothly varying E field around the density cutoff to limit vacuum fluctuations integrating into the plasma field. Since in the vacuum field the plasma E field should be zero except for some damping electric fields that show up near the edges of the plasma, then this gets integrated into B with the boundary conditions. E_ext is not subtracted off in this case as the plasma shielding response is just not accurate in the vacuum region. This really cuts down on the vacuum fluctuations for me and makes the simulations more stable.
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.
Sorry to get stuck on this more, but looking at it again a few hours after we talked I'm again unsure about the following:
You say the E-field from Ohm's law is the total field (i.e.
In such a case we'd have:
such that the full Faraday equation would give:
giving
If the Ohm's law calculated E field includes
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 terms of implementation, it seems to me that if the Ohm's law E-field does not already include
But the point of difference with the current implementation is, for clarity:
Is
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 am exploring options to make this better, but I think it would require either having a displacement solver that has slowed down EM waves, or the vector potential formulation with infinite speed propagation. This is more of a solution to quickly add fields to try out. I think this is better suited to deal with in a follow up PR. I think I need to do a bit of work on the Magnetostatic solver, then I can address doing a self-consistent magneto-inductive version of the hybrid algorithm.
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.
Why do you say
I think the implementation of the model as you propose is good, so I'll approve this PR and merge now. But I think we should continue a discussion of the accuracy of the underlying physics model (and adjust the implementation in future PRs if we agree it is needed).
@@ -604,7 +638,11 @@ void FiniteDifferenceSolver::HybridPICSolveECylindrical ( | |||
// interpolate the nodal neE values to the Yee grid | |||
auto enE_r = Interp(enE, nodal, Er_stag, coarsen, i, j, 0, 0); | |||
|
|||
Er(i, j, 0) = (enE_r - grad_Pe) / rho_val; | |||
if (rho_val < rho_floor && holmstrom_vacuum_region) { |
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's curious to me what the clang-tidy tests focus on. I would think it would prefer this to be
if (holmstrom_vacuum_region && (rho_val < rho_floor))
due to the possibility to short-circuit and avoid the comparison.
Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
… fields warning back to Initialization routine. Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
Signed-off-by: S. Eric Clark <25495882+clarkse@users.noreply.github.com>
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.
The code changes look good! Thanks for the all the work to get this implemented!
This PR allows for the addition of external fields through the particle fields analytical interface. This is useful for field splitting external vs. self fields in the hybrid ohm's law solver.