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

Implement geometry volumes for fiber grids in BEMC ScFi #370

Merged
merged 18 commits into from
Feb 22, 2023

Conversation

Chao1009
Copy link
Contributor

@Chao1009 Chao1009 commented Feb 19, 2023

Briefly, what does this PR introduce?

It adds geometry for grids so we can retrieve the position/dimension of a grid during the digitization/reconstruction.

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 updates the internal behavior of geometry construction for the BarrelCalorimeterInterlayers. It does not change any of the interfaces to use it.

Does this PR change default behavior?

No.

@Chao1009
Copy link
Contributor Author

Working on a visualization script for showing grid/fibers on local X-Z plane

@Chao1009 Chao1009 added the enhancement New feature or request label Feb 19, 2023
@Chao1009
Copy link
Contributor Author

It's a part of the solution to eic/EICrecon#499

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.

Some comments, more later.

src/BarrelCalorimeterInterlayers_geo.cpp Outdated Show resolved Hide resolved
src/BarrelCalorimeterInterlayers_geo.cpp Outdated Show resolved Hide resolved
src/BarrelCalorimeterInterlayers_geo.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot added the topic: infrastructure Regarding build system, CI, CD label Feb 19, 2023
@Chao1009 Chao1009 removed the topic: infrastructure Regarding build system, CI, CD label Feb 19, 2023
@github-actions github-actions bot added the topic: infrastructure Regarding build system, CI, CD label Feb 19, 2023
@Chao1009
Copy link
Contributor Author

The geometry visualization script is done
grid_fibers

@Chao1009
Copy link
Contributor Author

The same type of plot with different stave and layer (check title)
grid_fibers

@Chao1009 Chao1009 requested a review from wdconinc February 20, 2023 04:02
@Chao1009
Copy link
Contributor Author

Chao1009 commented Feb 20, 2023

Add more adjacent layers/grids in the visualization script to check the geometry.
grid_fibers

@Chao1009
Copy link
Contributor Author

Chao1009 commented Feb 20, 2023

This PR is probably ready, but I will test it with some changes for CalorimeterHitRec in EICRecon and then request to merge both of them together.

@Chao1009
Copy link
Contributor Author

Caught a minor issue with fiber alignment between adjacent layers. Fixed as
image

@wdconinc
Copy link
Contributor

@Chao1009 When you're ready, you should be able to click automerge and update the branch to get this merged.

@Chao1009 Chao1009 enabled auto-merge (squash) February 22, 2023 03:17
@Chao1009 Chao1009 merged commit 9a3029f into main Feb 22, 2023
@Chao1009 Chao1009 deleted the bemc-scfi-encode-grid-pos branch February 22, 2023 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request topic: barrel Mid-rapidity detectors topic: calorimetry topic: infrastructure Regarding build system, CI, CD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants