-
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
Add hybrid resistivity current term #4661
Add hybrid resistivity current term #4661
Conversation
for more information, see https://pre-commit.ci
…kse/WarpX into add_hybrid_resistivity_current_term
Real jx_val = Interp(Jx, Jx_stag, Ex_stag, coarsen, i, j, k, 0); | ||
Real jy_val = Interp(Jy, Jy_stag, Ex_stag, coarsen, i, j, k, 0); | ||
Real jz_val = Interp(Jz, Jz_stag, Ex_stag, coarsen, i, j, k, 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.
Real jx_val = Interp(Jx, Jx_stag, Ex_stag, coarsen, i, j, k, 0); | |
Real jy_val = Interp(Jy, Jy_stag, Ex_stag, coarsen, i, j, k, 0); | |
Real jz_val = Interp(Jz, Jz_stag, Ex_stag, coarsen, i, j, k, 0); | |
Real jx_val = Interp(Jx, Jx_stag, Ez_stag, coarsen, i, j, k, 0); | |
Real jy_val = Interp(Jy, Jy_stag, Ez_stag, coarsen, i, j, k, 0); | |
Real jz_val = Interp(Jz, Jz_stag, Ez_stag, coarsen, i, j, k, 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.
Thanks for catching the staggering bug.
Real jr_val = Interp(Jr, Jr_stag, Er_stag, coarsen, i, j, 0, 0); | ||
Real jt_val = Interp(Jt, Jt_stag, Er_stag, coarsen, i, j, 0, 0); | ||
Real jz_val = Interp(Jz, Jz_stag, Er_stag, coarsen, i, j, 0, 0); | ||
jtot_val = sqrt(pow(jr_val, 2) + pow(jt_val, 2) + pow(jz_val,2)); |
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 think the preferred way to calculate it would be:
jtot_val = sqrt(pow(jr_val, 2) + pow(jt_val, 2) + pow(jz_val,2)); | |
jtot_val = sqrt(jr_val*jr_val + jt_val*jt_val + jz_val*jz_val); |
and same for the other directions and Cartesian case.
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 have dropped using the pow call. Just curious though, with compiler optimization wouldn't pow(,2) be inlined anyway?
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.
Yes, with new enough compilers it should be but the reasoning is that with a simple square, the x*x
approach will never be less efficient than pow(x, 2)
whereas the latter could be less efficient with sub-optimal compiler flags.
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, actually with the change I don't think you need the static_cast<Real>
anymore if you use std::sqrt
, even in a lambda function now (#4490).
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.
Thanks for the check to only calculate j_tot
if it will be used.
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.
Thanks! I'll be curious to hear how you are using the current dependent resistivity.
This PR adds a current magnitude term in the hybrid resistivity to allow for scaling of the resistivity as a function of the magnitude of total J.