Density response #126
Density response #126
Conversation
| ! Test neutron density with different particle energies | ||
| p % type = P_NEUTRON | ||
| p % E = ONE | ||
| res = ONE/1.3831592645e+09_defReal |
There was a problem hiding this comment.
It is a bit annoying and I don't think there is reason to change it how it is.
It would be just clearer for the future people to either calculate reference from the formula for velocity or to include the one you used. along the lines of:
| res = ONE/1.3831592645e+09_defReal | |
| res = ONE / (sqrt(TWO * p % E)/ neutronMass) |
?
There was a problem hiding this comment.
Making a test using the same formula almost seems like cheating!
I could write the formula in a comment? Unless we might update the constants in the future, but I don't see why?
There was a problem hiding this comment.
Yeah... I had exactly the same pause. But really what the test checks is that right parameters are used for the right particle. There just isn't enough ways to calculate the velocity 😅
Formula in the comment is good!
As for the parameters we should really change them... at the moment they are taken from thin air. We should use the NIST CODATA values or the recommendations from the ENDF manual (the most recent version 2023, a lot of details do change silently between different editions I found out recently). And provide the source of the data in a comment.
| !! | ||
| !! The function returns the inverse of the velocity (response to score particle density) | ||
| !! if the particle type is neutron or photon, and zero otherwise | ||
| !! |
There was a problem hiding this comment.
| !! | |
| !! | |
| !! NOTE: | |
| !! The velocities are computed from non-relativistic formula for massive particles. | |
| !! The small error might appear in MeV range (e.g. for fusion applications) | |
| !! |
The difference is very small in this range, but could lead to bias with respect to other codes so worth to emphasise the approximation.
There was a problem hiding this comment.
One thing I might suggest is moving the speed calculation to the particle since we want it anyway (e.g., for alpha, dynamic), but this may not be necessary for the moment.
There was a problem hiding this comment.
Good point! A method in the particle may be the best place for the velocity indeed!
But I second that it is not necessary at the moment. We can marge this PR as it is and open an Issue about it. It would be a good 'first issue' for new contributors do deal with ;-)
There was a problem hiding this comment.
For me, I don't think that's necessary right now. It might clutter the particle and it won't be used for a long time. But it's a small change so I'll respect democracy.
There was a problem hiding this comment.
In the end I went for it, it was a very fast change. I believe I also addressed all the comments, so @ChasingNeutrons @Mikolaj-A-Kowalski let me know if I can go ahead and merge.
Adding a response that returns the inverse of the particle velocity to calculate the neutron density. Valid for neutrons and photons.