Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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