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

Sector Finder: Support for charged & neutral particles, added action function and validator #243

Merged
merged 17 commits into from
Jul 31, 2024

Conversation

rtysonCLAS12
Copy link
Contributor

Before this pull request the config file was set up to take the same bank to find sector for charged and uncharged particles. This is problematic because the natural choice for charged particles (DC) is different from neutrals (calorimeters). SectorFinder also needed an action function and a validator

This pulls request introduces:

  • Different banks for charged and neutral particles in config yaml. Choice is given to use default for one or both of charged/neutrals. Also fixed a bug due to certain banks containing non FD detectors, now only look at FD detectors.
  • Add pindex entry to output bank to avoid confusion due to filtering of REC::Particle bank.
  • Introduce a vector action function which finds the sector for a given pindex from a vector of sectors and pindices taken from some bank.
  • Introduce a validator. This produces plots of the per sector X/Y calorimeter position for e- and photons. It is then trivial to see that the sector finding works as the X/Y positions are all in the right sectors. See below.

image

@rtysonCLAS12
Copy link
Contributor Author

@c-dilks not sure why the CI is failing, sorry

@c-dilks
Copy link
Member

c-dilks commented Jul 11, 2024

@c-dilks not sure why the CI is failing, sorry

looks like a transient pip problem; successful after retry

Copy link
Member

@c-dilks c-dilks left a comment

Choose a reason for hiding this comment

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

Let's also make the action function's docstring clear to a new user (who may not want to read the code example, which I think is clear enough).

rtysonCLAS12 and others added 3 commits July 19, 2024 17:15
Co-authored-by: Christopher Dilks <c-dilks@users.noreply.github.com>
Co-authored-by: Christopher Dilks <c-dilks@users.noreply.github.com>
Co-authored-by: Christopher Dilks <c-dilks@users.noreply.github.com>
@rtysonCLAS12
Copy link
Contributor Author

Ok, thanks for the suggestions!

c-dilks
c-dilks previously approved these changes Jul 31, 2024
@c-dilks
Copy link
Member

c-dilks commented Jul 31, 2024

Ignore macOS CI failures, since the ROOT auto re-build over the weekend failed reproducibly.

@c-dilks c-dilks merged commit 2e7540c into main Jul 31, 2024
33 of 40 checks passed
@c-dilks c-dilks deleted the 237-sector-finder-for-uncharged branch July 31, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Sector Finder should allow for separate user defined banks for charged and uncharged particles.
2 participants