Skip to content
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

Feature/limited light backtracking with MAX_MC_TRUTH_IDS = 3 and MC_LIGHT_THRESHOLD=1 #175

Merged
merged 11 commits into from
Dec 16, 2023

Conversation

marjoleinvannuland
Copy link
Contributor

Very limited light backtracking. Per light detector per tick true photons per segment are saved. The maximum number of segments per light detector per tick is 3. Only light signals larger than 1 pe are accepted. The segments are sorted in order of the number of photons they contribute to a light detector, so we get only the most relevant segments.
NB. these settings cause unpredictable cuda memory errors, setting MAX_MC_TRUTH_IDS=0 will disable the light truth info.

@YifanC
Copy link
Collaborator

YifanC commented Dec 16, 2023

Hi @marjoleinvannuland , Thank you for the PR.
Some very minor comments. I hope it's not too late.

  1. To be able to utilise the default values in "larndsim/consts/light.py", I would suggest something like following:
    MAX_MC_TRUTH_IDS = detprop.get('max_light_truth_ids',MAX_MC_TRUTH_IDS)
    MC_TRUTH_THRESHOLD = detprop.get('mc_truth_threshold',MC_TRUTH_THRESHOLD)
    and maybe also add MC_TRUTH_THRESHOLD as a global variable.
  2. I would suggest to keep the default values for MAX_MC_TRUTH_IDS and MC_TRUTH_THRESHOLD as it is in "larndsim/consts/light.py", but add them in the yaml in the simulation_properties. Therefore the parameter adjustment would be in the configuration file rather hardcoded in the script.

I will follow up on the waveform validation plot on slack, but I think with the above two minor comments addressed. It should be ready to go.

Copy link
Member

@krwood krwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good by eye. Let's merge in for the MiniRun5_beta production over the weekend, and we can take a look again next week.

@krwood krwood merged commit 92d60ab into develop Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants