-
Notifications
You must be signed in to change notification settings - Fork 29
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
Light Sim Addition #39
Conversation
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 left some comments in line, mainly about hardcoded numbers and documentation. Thanks!
larndsim/lightLUT.py
Outdated
def GetVoxel(pos,lut_geometry): # Determines which voxel is being called based on the position of the edep. Voxels are indexed 0-2911 | ||
|
||
(lut_min,lut_max,lut_ndiv) = lut_geometry | ||
vox_xyz = np.floor(pos/(lut_max-lut_min)*lut_ndiv).astype(int)+lut_ndiv/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 don't quite understand here, you apply the floor function, then cast to integer and then add a float? What's a typical value for lut_ndiv
?
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.
this function now gets the voxel from the global coordinate system with a single int call (see https://github.com/tairaeli/larnd-sim/blob/larnd-branch/larndsim/lightLUT.py#L33)
…ell as some of the comments on some of the lines of code that were not clearly explained
…the removal of some unnecessary code
…Not complete yet. Still need to be fully integrated into the rest of the lightLUT
…into larnd-branch
…n. Also removed the get_lut_geometry function and added the lut geometries to the module0.yaml file. Testing still required
…ulation. Fixed errors in coordinate translation
…based on tpc_borders
…into larnd-branch
…d random "out of bounds" edeps
…into larnd-branch
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 PR looks very good. I just have a couple of minor comments that should be quick to address. Thank you for your patience.
…into larnd-branch
Light Sim Addition
Added a light simulation model relying on a light lookup table. Made some additions to a few of the other modules to account for these additions.