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 t0 vertex #140

Merged
merged 6 commits into from
Jun 11, 2023
Merged

Feature t0 vertex #140

merged 6 commits into from
Jun 11, 2023

Conversation

YifanC
Copy link
Collaborator

@YifanC YifanC commented Jun 2, 2023

No description provided.

@YifanC YifanC requested review from mjkramer and krwood June 2, 2023 07:06
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.

@YifanC and I tested this this new feature by running the simulation on an events with segments ("tracks" in larnd-sim) that saturate the the MAX_TRACKS_PER_PIXEL configuration variable. The situation is handled much more sensibly with this PR.

  • Before: all of the fractional contributions sum up to 1 by construction
  • After: a pixel that has been "saturated" in the simulation would be indicated by the fractional contributions not summing up to 1

This is a very similar solution to Issue #137 as Pull Request #138, but is also robust to the randomness from the noise simulation. Merging this PR should also close both #137 and #138.

Comment on lines +453 to +455
#tot_backtracked = 0
#for itrk in range(current_fractions.shape[2]):
# tot_backtracked += current_fractions[ip][iadc][itrk]
Copy link
Member

Choose a reason for hiding this comment

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

[nit-picky comment alert] Might as well remove the commented out, legacy code

@YifanC YifanC merged commit b7f71a6 into develop Jun 11, 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.

4 participants