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

add position masks for hit reco #500

Merged
merged 2 commits into from
Feb 22, 2023
Merged

add position masks for hit reco #500

merged 2 commits into from
Feb 22, 2023

Conversation

Chao1009
Copy link
Contributor

Briefly, what does this PR introduce?

It's part of the solution to #499
related to eic/epic#370

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No, it adds an option to use the mother volume's position of a hit

Does this PR change default behavior?

No, by default this option is turned off.

@Chao1009 Chao1009 self-assigned this Feb 21, 2023
@Chao1009 Chao1009 marked this pull request as draft February 21, 2023 16:39
@Chao1009 Chao1009 marked this pull request as ready for review February 22, 2023 01:22
Copy link
Contributor

@wdconinc wdconinc left a comment

Choose a reason for hiding this comment

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

Changes look good to me.

But should we propate algorithm option parsing to all factories that inherit from it? @veprbl @DraTeots @faustus123

@Chao1009 Chao1009 enabled auto-merge February 22, 2023 03:17
@Chao1009 Chao1009 merged commit 59a0526 into main Feb 22, 2023
@Chao1009 Chao1009 deleted the calhitreco_add_fields_mask branch February 22, 2023 04:19
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.

2 participants