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

Fix: r Default in AddPlasmaFlux #5704

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Feb 24, 2025

It appears to me that in the case of EBs with inject_from_eb the variable r.y is used as undefined in case that we do not randomize the theta.

See: line 1666ff. below the line I changed for the use of r.y in the case of multiple modes and/or non-random theta:

                const Real theta = (nmodes == 1 && rz_random_theta)?
                    (2._prt*MathConst::pi*amrex::Random(engine)):
                    (2._prt*MathConst::pi*r.y);

What would be a good default value? I think zero might actually not what we want here... but maybe something depending on pos in that case.

It appears to me that in the case of no-EBs or EBs
with
`!inject_from_eb` the variable `r.y` is used as
undefined (often zero).

What would be a good default value?
@ax3l ax3l added bug Something isn't working bug: affects latest release Bug also exists in latest release version geometry: RZ axisymmetric 2D and quasi-3D component: initialization Changes related to the initialization of the simulation labels Feb 24, 2025
@ax3l ax3l requested review from dpgrote and RemiLehe February 24, 2025 23:22
@ax3l ax3l added the component: boundary PML, embedded boundaries, et al. label Feb 24, 2025
@ax3l ax3l assigned RemiLehe and unassigned RemiLehe Feb 24, 2025
@dpgrote
Copy link
Member

dpgrote commented Feb 24, 2025

Good catch. I think in this case, it might be better to set r.y = amrex::Random(engine) with EB and inject_from_eb. The r.y is setting the theta and it would be good to have this uniformly distributed. Another option would be to take the setting of r outside of the else block and always call getPositionUnitBox. This would allow the user to select between uniformly spaced theta and random theta.

@@ -1581,7 +1581,7 @@ PhysicalParticleContainer::AddPlasmaFlux (PlasmaInjector const& plasma_injector,

// Determine the position of the particle within the cell
XDim3 pos;
XDim3 r;
XDim3 r = {0._rt, 0._rt, 0._rt};
Copy link
Member Author

Choose a reason for hiding this comment

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

Not the solution. I think we need to set r to pos inside if (inject_from_eb) or so

@ax3l ax3l added the help wanted Extra attention is needed label Feb 25, 2025
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: boundary PML, embedded boundaries, et al. component: initialization Changes related to the initialization of the simulation geometry: RZ axisymmetric 2D and quasi-3D help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants