-
Notifications
You must be signed in to change notification settings - Fork 191
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: Ablastr PoissonSolver Constant #4090
Fix: Ablastr PoissonSolver Constant #4090
Conversation
Seen in ImpactX: ``` build/_deps/fetchedablastr-src/Source/ablastr/fields/PoissonSolver.H:163:31: error: use of undeclared identifier 'PhysConst' rho[lev]->mult(-1._rt/PhysConst::ep0); // TODO: when do we "un-multiply" this? We need to document this side-effect! ^ ```
Workaround for ECP-WarpX/WarpX#4090 in ABLASTR 23.07
* Release 23.07 Prepare the July release of ImpactX. * FIx: PoissonSolve.cpp Constant ``` impactx/src/particles/spacecharge/PoissonSolve.cpp:106:36: error: use of undeclared identifier 'PhysConst' rho[lev].mult(-1._rt * PhysConst::ep0); ^ ``` * [Workaround] ABLASTR 23.07 Poisson eps0 Workaround for ECP-WarpX/WarpX#4090 in ABLASTR 23.07
Remove the workaround for ECP-WarpX/WarpX#4090
@@ -160,7 +160,8 @@ computePhi (amrex::Vector<amrex::MultiFab*> const & rho, | |||
// scale rho appropriately; also determine if rho is zero everywhere | |||
amrex::Real max_norm_b = 0.0; | |||
for (int lev=0; lev<=finest_level; lev++) { | |||
rho[lev]->mult(-1._rt/PhysConst::ep0); // TODO: when do we "un-multiply" this? We need to document this side-effect! | |||
using namespace ablastr::constant::SI; |
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.
Does the using-declaration need to be inside the for
loop over levels?
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 would suggest placing the using
declaration at the beginning of the function rather then in the for loop.
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 using declaration is performance neutral and the only type we need is inside this loop. Thus, I would not pollute the whole scope of the body of the function just yet and keep it localized here.
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.
Ok! you have convinced me
Remove the workaround for ECP-WarpX/WarpX#4090
Remove the workaround for ECP-WarpX/WarpX#4090
* Release 23.08 Prepare the August release of ImpactX. * PoissonSolve: Remove ABLASTR Workaround Remove the workaround for ECP-WarpX/WarpX#4090 * Examples: New AMReX Multi-Line Continuation
Seen in ImpactX: