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

Fixing bug in hyper-resistivity calculation which had missing terms i… #5638

Conversation

clarkse
Copy link
Member

@clarkse clarkse commented Feb 4, 2025

…n vector laplacian evaluation. Additioanally fixing a staggering bug for density calculation in RZ.

…n vector laplacian evaluation. Additioanally fixing a staggering bug for density calculation in RZ.

Signed-off-by: S. Eric Clark <[email protected]>
Signed-off-by: S. Eric Clark <[email protected]>
Copy link
Member

@roelof-groenewald roelof-groenewald left a comment

Choose a reason for hiding this comment

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

Hmm, the hyper-resistivity term is a vector Laplacian, no? I agree there are bugs here, but it looks like you changed it to a sort-of hybrid scalar / vector Laplacian. Or am I missing something?

For RZ the individual components should be as follows:
image

But the Cartesian implementation was already correct.

Update: Nevermind. I see this is in fact all wrong. Thanks for catching!

@@ -633,13 +635,14 @@ void FiniteDifferenceSolver::HybridPICSolveECylindrical (
}

// Interpolate to get the appropriate charge density in space
Real rho_val = Interp(rho, nodal, Er_stag, coarsen, i, j, 0, 0);
Real rho_val = Interp(rho, nodal, Et_stag, coarsen, i, j, 0, 0);
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch!

@ax3l ax3l added the bug Something isn't working label Feb 4, 2025
@ax3l ax3l added the bug: affects latest release Bug also exists in latest release version label Feb 4, 2025
@roelof-groenewald roelof-groenewald added the component: fluid-ohm Related to the Ohm's law solver (with fluid electrons) label Feb 4, 2025
@roelof-groenewald
Copy link
Member

The fixes look good! Just the one comment above about the resistivity_has_J_dependence issue and this should be good to go!

…k dependencies on flags for standard resistivity.

Signed-off-by: S. Eric Clark <[email protected]>
Copy link
Member

@roelof-groenewald roelof-groenewald left a comment

Choose a reason for hiding this comment

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

Thanks so much for fixing!

@roelof-groenewald roelof-groenewald enabled auto-merge (squash) February 4, 2025 18:58
auto-merge was automatically disabled February 4, 2025 19:49

Head branch was pushed to by a user without write access

@roelof-groenewald roelof-groenewald enabled auto-merge (squash) February 4, 2025 19:54
@roelof-groenewald roelof-groenewald merged commit 12269a0 into ECP-WarpX:development Feb 4, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: affects latest release Bug also exists in latest release version bug Something isn't working component: fluid-ohm Related to the Ohm's law solver (with fluid electrons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants