-
Notifications
You must be signed in to change notification settings - Fork 35
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 Wentzel macro xs calculator and fix a_sq_factor
#1274
Add Wentzel macro xs calculator and fix a_sq_factor
#1274
Conversation
0fbb7dc
to
8a51eca
Compare
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.
LGTM, but I'll let @whokion take a look before merging. I'm going to remove the bug
label since this is mostly an enhancement and the bug hasn't been seen in non-testing use cases.
@@ -36,7 +36,7 @@ struct CoulombParameters | |||
//! Factor used to calculate the maximum scattering angle off of a nucleus | |||
real_type a_sq_factor{real_type(0.5) | |||
* ipow<2>(constants::hbar_planck * constants::c_light | |||
* units::femtometer)}; | |||
/ units::femtometer)}; |
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.
Maybe a Quantity
(FmSq?) would help elucidate this factor? The divide vs multiply has bitten me several times...
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.
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.
Mo problem, I will take a second look for this today.
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.
Okay, it looks all good to me. This factor is related tothe screening parameter
, screening radius
in terms of the Bohr radius
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 explanation and reference! That makes more sense now. I'll take another look and see if I can understand the differences between the screening parameter and the value that's used in the code to limit the angle.
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 I fixed another bug (in the calculation of the effective atomic mass^(-2/3)), but the quantity that we're subtracting here is still about 4 orders of magnitude smaller than the screening parameter. Should they be roughly the same? The screening parameter itself is also very small (like O(1e-10)) which means the maximum angle is going to be limited to near zero instead of the polar angle limit... so I think I'm still missing something.
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, I guess that the magnitude, 1-factorA^2/ [(mass number)^{2/3} * p^2] should be close to 1, but not too close to 1 - based on my rough calculation, it is about 0.95/0.98 for electrons of 100/200 MeV (kinetic) energy on Pb (A~200).
@@ -77,7 +77,7 @@ WentzelOKVIParams::WentzelOKVIParams(SPConstMaterials materials, | |||
host_data.params.a_sq_factor | |||
= real_type(0.5) | |||
* ipow<2>(options.angle_limit_factor * constants::hbar_planck | |||
* constants::c_light * units::femtometer); | |||
* constants::c_light / units::femtometer); |
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.
Since the factor's always being set here (it looks like??) maybe just default the CoulombParameters
to zero and reduce the duplication 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.
LGTM too.
One more fix... I think |
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.
Good work!
a_sq_factor
for Wentzel OK&VIinv_mass_cbrt_sq
a_sq_factor
units